-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to fit sed with G23 only #124
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. A couple of changes needed.
measure_extinction/modeldata.py
Outdated
axav = g23mod(shifted_waves) | ||
ext_sed[cspec] = sed[cspec] * (10 ** (-0.4 * axav * params[0])) | ||
|
||
elif fit_range == "all": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having this as the "elif", should it be "else"? In other words, what happens if someone passes fit_range = "all_butItypeitwrong"? Or an "else" statement needs to be added to handle the case with fit_range is not "all" or "g23".
Codestyle errors need to be fixed. Can run |
doc test error is fixed in PR #123 |
Following updates have been made: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The full pull request includes two commits.
The first commit adds an argument to
dust_extinguished_sed()
function inmodel_data.py:212
, to take in a keyword that allows the sed to only be fit withG23()
.dust_extinguished_sed()
is called bylnlike()
infit_model.py:88
, which in turn is called bylnprob()
infit_model.py:151
both
lnlike()
andlnprob()
are updated to take in the new keyword "g23" as an optional argument. These were made optional arguments as not to break anything that continues to use these functions without the new argument. The default keyword is set to "all", which executes the original sed fitting indust_extinguished_sed()
, rather thanG23
only. The original sed fitting, done using the_curve_F99_method()
can be called by excluding the new argument, when callinglnlike()
,lnprob()
, and/ordust_extinguished_sed()
.The second commit adds a warning if mismatched band information is provided, between the model and reddened star data.