kevin218 / Eureka

Eureka! is a data reduction and analysis pipeline intended for time-series observations with JWST.
https://eurekadocs.readthedocs.io/
MIT License
56 stars 43 forks source link

Adding _ch to channel tag and _pl to planet tag in S5 variable names #659

Open taylorbell57 opened 3 weeks ago

taylorbell57 commented 3 weeks ago

This PR reformats the fitted variable names in Stage 5 to use _ch# for different channels and _pl# for different planets. This will make the code far more robust since we can far more easily identify the channel and planet number, and this should also help with Stage 6 bugs. It will also make the variable names a bit more human readable, and this PR should also fix multiple bugs related to trying to manually set different priors or fixed values for different channels.

These changes resolve #643

While I was at it, I also added support for ecosw and esinw in astrophysical models which avoids the well known ecc>0 bias of fitting for ecc and w directly.

I still need to fully integrate and test the _pl tags, especially for starry, and I still need to do a bunch of testing. For now I'm just opening a draft PR to help me keep track of my progress, so don't worry about reviewing it until I mark it as ready for review.

Note, this change is going to break backwards compatibility for S6 running on old S5 outputs, but that isn't a huge deal since S6 isn't critical and we're working towards v1.0

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 38.50267% with 345 lines in your changes missing coverage. Please review.

Project coverage is 54.01%. Comparing base (a939236) to head (caf5823).

Files Patch % Lines
src/eureka/S6_planet_spectra/s6_spectra.py 47.77% 94 Missing :warning:
...curve_fitting/differentiable_models/PyMC3Models.py 1.40% 70 Missing :warning:
...ightcurve_fitting/differentiable_models/GPModel.py 5.47% 69 Missing :warning:
...ureka/S5_lightcurve_fitting/models/BatmanModels.py 61.01% 23 Missing :warning:
...curve_fitting/differentiable_models/StarryModel.py 0.00% 10 Missing :warning:
...c/eureka/S5_lightcurve_fitting/gradient_fitters.py 9.09% 10 Missing :warning:
src/eureka/S5_lightcurve_fitting/lightcurve.py 57.14% 6 Missing :warning:
src/eureka/S5_lightcurve_fitting/likelihood.py 62.50% 6 Missing :warning:
...rve_fitting/differentiable_models/CentroidModel.py 16.66% 5 Missing :warning:
...urve_fitting/differentiable_models/ExpRampModel.py 16.66% 5 Missing :warning:
... and 14 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #659 +/- ## ========================================== - Coverage 54.24% 54.01% -0.23% ========================================== Files 101 101 Lines 12793 12942 +149 ========================================== + Hits 6939 6991 +52 - Misses 5854 5951 +97 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

taylorbell57 commented 2 weeks ago

Alright, this PR is now known to work well through to Stage 5, but I haven't yet tested to see what impact this has on Stage 6. My suspicion is that Stage 6 will merge together the spectra of all planets, so I'll likely need to address that