Skip to content

Added a utility function to copy a model with new parameter values#16806

Closed
astrofrog wants to merge 3 commits intoastropy:mainfrom
astrofrog:copy-model-with-new-parameter-values
Closed

Added a utility function to copy a model with new parameter values#16806
astrofrog wants to merge 3 commits intoastropy:mainfrom
astrofrog:copy-model-with-new-parameter-values

Conversation

@astrofrog
Copy link
Member

@astrofrog astrofrog commented Aug 6, 2024 •
edited
Loading

This is an experimental PR which aims to solve #16593 and is an alternative to #16593

With this PR, we can copy a model, replacing the parameter values with new ones. If the new parameter values are arrays, all existing parameters are also broadcasted to the same array shape. Example usage:

In [1]: from astropy.modeling.models import Gaussian1D

In [2]: g = Gaussian1D()

In [3]: g
Out[3]: <Gaussian1D(amplitude=1., mean=0., stddev=1.)>

In [4]: from astropy.modeling.utils import copy_with_new_parameter_values

In [5]: import numpy as np

In [6]: g2 = copy_with_new_parameter_values(g, mean=np.array([[1, 2, 3], [4, 5, 6]]))

In [7]: g2
Out[7]: <Gaussian1D(amplitude=[[1., 1., 1.], [1., 1., 1.]], mean=[[1., 2., 3.], [4., 5., 6.]], stddev=[[1., 1., 1.], [1., 1., 1.]])>

In [8]: g3 = copy_with_new_parameter_values(g, mean=1, amplitude=2, stddev=3)

In [9]: g3
Out[9]: <Gaussian1D(amplitude=2., mean=1., stddev=3.)>

Putting this up for discussion with @Cadair, @nden and @perrygreenfield

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2024

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2024

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

Cadair
Cadair reviewed Aug 6, 2024
View reviewed changes
astropy/modeling/utils.py
Comment on lines +373 to +376
# HACK: for some models, there are additional non-parameter arguments.
# We hard-code the fix for some models.
if isinstance(model, (models.Polynomial1D, models.Polynomial2D)):
args = (model.degree,)
Copy link
Member

@Cadair Cadair Aug 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the major problem with this approach. We can't hard code these attributes for all models (I have custom DKIST ones for example), so we would need to generalise this or come up with a different solution.

@pllim pllim added this to the v7.0.0 milestone Aug 6, 2024
@astrofrog astrofrog mentioned this pull request Aug 9, 2024
Merged
1 task
@nden nden closed this in #16812 Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Cadair Cadair Cadair left review comments

Assignees

No one assigned

Labels

modeling

Projects

None yet

Milestone

v7.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@astrofrog @Cadair @pllim