grd349 / PBjam

A repo for our peak baggin code and tips on jam
MIT License
17 stars 6 forks source link

Teff and bp_rp are now optional inputs #241

Closed nielsenmb closed 4 years ago

nielsenmb commented 4 years ago

At least one must be provided though.

The missing parameters are just replaced by the mean of the prior and a very wide std before it gets fed to KDE.

Also added unit tests for these parts.

Also added developer mode for star, session and asy_peakbag, this will add the input dnu and numax as additional priors in asy_peakbag. This is normally turned off to avoid double counting, but for the purposes of expanding the prior it will probably help. Should be turned off for science.

nielsenmb commented 4 years ago

@grd349 ready to merge

nielsenmb commented 4 years ago

At 2 sigma, what I put in covers the 16500 targets in Teff by a margin of 800K, the margin might be a bit narrower for bp_rp.

It seems to work on RGs though. Haven't run it on an F-star yet.

For now I don't really have an issue with asking people to supply both if they can. Soon PBjam will automatically get bp_rp anyway, so Teff will be the only thing they have to worry about.

On Mon, 8 Jun 2020 at 07:38, Guy Davies notifications@github.com wrote:

@grd349 commented on this pull request.

Looks fine to me. Is this really the functionality we want? What happens when someone runs on a bloody F-star - The new prior will prefer lower temperatures. If we force people to supply values then they must think about what they are doing. The prior on Teff for a main sequence star with seismic detection is probably more like 6000 \pm 1000.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/241#pullrequestreview-425947782, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO37HMIOZKCKAWKEWAZTRVSBPVANCNFSM4NTOE3ZA .

nielsenmb commented 4 years ago

@grd349 should be ready to merge this one if you're happy with it.

grd349 commented 4 years ago

Merged!