Skip to content

Include sigma av as another parameter for starwave to use. - #25

Open
joycelin1123 wants to merge 3 commits into
vedantchandra:masterfrom
joycelin1123:include_sigma_av
Open

Include sigma av as another parameter for starwave to use.#25
joycelin1123 wants to merge 3 commits into
vedantchandra:masterfrom
joycelin1123:include_sigma_av

Conversation

@joycelin1123

Copy link
Copy Markdown
Contributor

Add in sig_av as another parameter. In starwave.py, get_cmd and sample_cmd have been updated to include this change. In parameters.py, make_params has been updated with an extra line to include this.

Add in sig_av in get_cmd and sample_cmd as a new parameter for starwave to use.
Add in sig_av as a parameter in the make_params function
@mgennaro

Copy link
Copy Markdown
Collaborator

It would seem that this PR also includes the export of other quantities (sdict) from the get_cmd method. Please clean that up and submit a separate PR for that, so it's documented.
Also, please check what happens if one sets the sigma_av to 0. That will be a typical use case (i.e. no differential reddening) and it would be nice to be able to fallback to the original behavior of no sigma_av. If sigma_av == 0 causes problems with the normal distribution being degenerate/undefined, then please set the default value to something small (eg 1e-5) which is effectively zero, but first check that such small number indeed does not cause problems with the normal distribution.

@mgennaro

mgennaro commented Oct 3, 2024

Copy link
Copy Markdown
Collaborator

Looking a little closer into this PR. While it is a good idea to have a "sigma_av" to allow for (a gaussian form of) differential reddening, I would suggest withdrawing it and rearranging things to use a truncated gaussian, in order to avoid the extinction from assuming negative values: https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.truncnorm.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants