kevin218 / Eureka

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

Allowing joint fits of multiple white-light observations and patching bugs with joint fits #510

Closed taylorbell57 closed 1 year ago

taylorbell57 commented 1 year ago

This PR resolves #499.

This PR resovles several bugs in the code that allows joint fitting of spectroscopic channels (e.g. simultaneously fit all 14 MIRI/LRS binned wavelengths). This PR also enables the simultaneous fitting of multiple observations from different times and/or instruments (e.g. repeated visits of one target, or white lightcurves from multiple instruments). This is great for determining the "best" orbital parameters to use for a system when looking at spectra from multiple instruments or repeated visits of target (e.g. the TRAPPIST-1b photometry I was working on).

The beginnings of the code were started by Erin May, and I then copy-pasted those edits many months ago because I couldn't figure out how to merge code from her fork into my branch. I since figured out how to do that and have done my best to give credit where it is due.

The flake8 and pytests both run successfully on my laptop.

I'd request that after this PR gets merged we increment the version to v0.9 (so that I can cite this version in a couple of upcoming papers).

codecov-commenter commented 1 year ago

Codecov Report

Merging #510 (7f9c603) into main (eb6ced2) will decrease coverage by 0.43%. The diff coverage is 55.82%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #510      +/-   ##
==========================================
- Coverage   58.60%   58.17%   -0.43%     
==========================================
  Files          90       91       +1     
  Lines       10465    10877     +412     
==========================================
+ Hits         6133     6328     +195     
- Misses       4332     4549     +217     
Impacted Files Coverage Δ
src/eureka/S5_lightcurve_fitting/models/GPModel.py 9.18% <ø> (-0.19%) :arrow_down:
...htcurve_fitting/differentiable_models/StepModel.py 24.00% <4.76%> (-2.20%) :arrow_down:
...rve_fitting/differentiable_models/CentroidModel.py 30.18% <5.55%> (-3.91%) :arrow_down:
...reka/S5_lightcurve_fitting/models/CentroidModel.py 21.27% <7.40%> (-7.30%) :arrow_down:
...itting/differentiable_models/SinusoidPhaseCurve.py 19.54% <10.00%> (+0.16%) :arrow_up:
...c/eureka/S5_lightcurve_fitting/models/StepModel.py 17.39% <12.50%> (-0.56%) :arrow_down:
src/eureka/S5_lightcurve_fitting/s5_fit.py 68.71% <39.74%> (-7.69%) :arrow_down:
src/eureka/lib/astropytable.py 61.53% <42.85%> (-9.90%) :arrow_down:
src/eureka/S5_lightcurve_fitting/fitters.py 64.98% <46.34%> (-0.97%) :arrow_down:
...curve_fitting/differentiable_models/StarryModel.py 73.18% <52.38%> (-6.27%) :arrow_down:
... and 16 more

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

taylorbell57 commented 1 year ago

One nuance that this PR doesn't address is that currently all systematics models are applied to all datasets. This is fine if you're fitting repeated observations of a target with the same instrument since you're likely going to want the same model for all the observations, but if you're fitting a NIRCam white lightcurve and a MIRI white lightcurve, then you're forced to fit an exponential ramp to the NIRCam data while you only want to fit it to the MIRI data.

I think this nuance should be left for a future PR though, given how large this PR already is and how helpful it already is.