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

Enable Binned LC Plots #512

Closed kevin218 closed 1 year ago

kevin218 commented 1 year ago

Working with datasets that have many integrations makes it difficult to see what's going on in the light curve plots. So, when meta.nbin_plot is given, the light curves in all figures are now binned to the desired time resolution. Before, only some plots were binned.

I also updated the jwst version to 1.8.2. Version 1.9 appears to still be buggy.

taylorbell57 commented 1 year ago

I'm also going to push some minor changes to this PR (hope you don't mind). I've just merged in the new main branch (so I can test the code locally with all my recent bug patches), and made some very minor changes to figures 5104 and 5304 so that they actually get triggered with the PyMC3 fitters when fitting phase variations with the sinusoid_pc model instead of starry's spherical harmonics model.

codecov-commenter commented 1 year ago

Codecov Report

Merging #512 (4a971ed) into main (a3ca120) will decrease coverage by 0.11%. The diff coverage is 52.00%.

: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     #512      +/-   ##
==========================================
- Coverage   58.18%   58.08%   -0.11%     
==========================================
  Files          91       91              
  Lines       10899    10937      +38     
==========================================
+ Hits         6342     6353      +11     
- Misses       4557     4584      +27     
Impacted Files Coverage Δ
src/eureka/S5_lightcurve_fitting/likelihood.py 68.68% <ø> (ø)
src/eureka/S5_lightcurve_fitting/lightcurve.py 73.28% <30.30%> (-14.84%) :arrow_down:
src/eureka/S5_lightcurve_fitting/plots_s5.py 79.88% <92.30%> (+0.17%) :arrow_up:
...curve_fitting/differentiable_models/PyMC3Models.py 70.06% <100.00%> (ø)
...c/eureka/S5_lightcurve_fitting/gradient_fitters.py 85.40% <100.00%> (ø)
src/eureka/S5_lightcurve_fitting/models/Model.py 77.25% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

kevin218 commented 1 year ago

I'm going on vacation until April 12th, so you're welcome to make changes to this PR while I'm gone.

I support having Figures 5104 and 5304 as separate plots.

taylorbell57 commented 1 year ago

Can you clarify what you mean by "I support having Figures 5104 and 5304 as separate plots."? To clarify my question, I had meant do we want to:

  1. have fig5104 (just binned data) and fig5304 (binned data with unbinned data in the background) equivalents for all the figures you've changed, or
  2. just have the fig5014 (just binned data) equivalents that you've made
kevin218 commented 1 year ago

Ok, now I see what you're asking.

For the 3 panel plot (raw, normalized, residuals), I would prefer using only the binned data so that the default ylim range gives meaningful information.

I'm ok with plotting both for the single-panel light curve.

taylorbell57 commented 1 year ago

Okay, I'll try to make both for the single-panel lightcurve if I find the time before you get back.