Pass SpectralAxis to physical models rather than energy edges#223
Open
jajmitchell wants to merge 29 commits intosunpy:mainfrom
Open
Pass SpectralAxis to physical models rather than energy edges#223jajmitchell wants to merge 29 commits intosunpy:mainfrom
jajmitchell wants to merge 29 commits intosunpy:mainfrom
Conversation
Adds SRM units and passes Spectralaxis to thermal
Checks input is SpectralAxis object, converts and warns if not
Adds check input function
ThickTarget and ThinTarget use centers
Albedo now takes SpectralAxis and centers
Pre-commit
scaling.py and models.py now function with centers
Fixes scaling and albedo tests
Adds __array_finalize__ method to SpectralAxis
Fixes tests
Fixes part of fitting_simulated_data.py
Reverts SRM model to main version
Fixes fitting tests
Fixes test_models.py
Fixes Albedo example
Fix Albedo example 2
Fixes examples
Adds changelog
Pre_commit fix
Member
|
Nice is this ready for review? |
Contributor
Author
@samaloney I need to just check that the fitting works with the physical models, then I think its good to go |
Contributor
Author
|
@samaloney the complications continue. The issue comes when fitting, specifically in Astropy fitting.py line 255, as can be seen below. This means that for fitting, before being passed to evaluate, the fitter takes the value of the SpectralAxis object, which not only strips the units, but also all other attributes. I'm trying to think of ways around this, but haven't come up with anything yet. |
Adds custom `__call__` to `ThermalEmission`
Fixes test_thermal.py
Contributor
Author
|
I have added a custom |
Adds meta to SpectralAxis, thermal model can now take Spectrum object, Spectral axis, or array
Can pass array without initialising
Fitting works
Remove unused functions
Fixes continuum and line models
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR enables the physical models to be used with a SpectralAxis
By passing a SpectralAxis to the physical models, they are effectively evaluated at bin centers, but have access to the bin edges as these are stored as an attribute in the SpectralAxis class. This change will allow for a clearer path to using Astropy model classes for fitting as the dimensionality of the input and output will no longer be an issue.