legend-exp / pygama

Python package for data processing and analysis
https://pygama.readthedocs.io
GNU General Public License v3.0
18 stars 56 forks source link

Pargen refactor #564

Closed ggmarshall closed 4 months ago

ggmarshall commented 6 months ago
sagitta42 commented 6 months ago

Hi there, I'm already looking at some changes in this PR to note changes that I need to pay attention to in my code that uses classes and functions from pargen.

One thing that I noticed:

Before: dep_acc is a member of the cal_aoe class passed to init; in cal_aoe.calibrate() we call cal_aoe.get_aoe_cut_fit() which has an input variable dep_acc where we pass a hard coded calue of 0.9; which is however not used, self.dep_acc is used inside that function.

Now: dep_acc is a member of the CalAoE class passed to init; but also an input variable in CalAoE.calibrate() which it then passes it to CalAoE.get_aoe_cut_fit(), but self.dep_acc is still used there anyway, not dep_acc passed by calibrate().

It wouldn't break anything since it's always 0.9 as of now, but wanted to note this just in case, also from the point of view of pars_hit_aoe.py later.

sagitta42 commented 6 months ago

Another question:

I see that ecal_th.py is deleted entirely. I imagine there would be changes to the flow in pars_hit_ecal.py in a corresponding PR in legend-dataflow? Where did the apply_cuts and calibrate_parameter functionality get moved? (sorry, I can't seem to understand that from digging around in this PR) I am currently using both, following the procedure in pars_hit_ecal.py.

ggmarshall commented 6 months ago

I'm still working my way up through the chain so haven't got to a/e, lq yet, the dataflow changes are coming but again I have been working on up to dsp and haven't finished higher tiers yet

sagitta42 commented 6 months ago

I see, thanks! I'll stay tuned then and prepare for changes :)

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 47.46060% with 1200 lines in your changes are missing coverage. Please review.

Project coverage is 52.43%. Comparing base (ae3752a) to head (7c0de64). Report is 78 commits behind head on refactor.

:exclamation: Current head 7c0de64 differs from pull request most recent head 2c79cd1. Consider uploading reports for the commit 2c79cd1 to get more accurate results

Files Patch % Lines
src/pygama/pargen/dsp_optimize.py 0.00% 299 Missing :warning:
src/pygama/pargen/data_cleaning.py 40.52% 160 Missing :warning:
src/pygama/pargen/energy_optimisation.py 4.24% 158 Missing :warning:
src/pygama/pargen/extract_tau.py 0.00% 111 Missing :warning:
src/pygama/pargen/dplms_ge_dict.py 0.00% 56 Missing :warning:
src/pygama/math/hpge_peak_fitting.py 66.10% 40 Missing :warning:
src/pygama/pargen/lq_cal.py 25.00% 39 Missing :warning:
src/pygama/math/functions/exgauss.py 56.32% 38 Missing :warning:
src/pygama/math/functions/sum_dists.py 84.58% 37 Missing :warning:
src/pygama/math/functions/step.py 62.02% 30 Missing :warning:
... and 16 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## refactor #564 +/- ## ============================================= + Coverage 22.82% 52.43% +29.61% ============================================= Files 42 64 +22 Lines 8110 7552 -558 ============================================= + Hits 1851 3960 +2109 + Misses 6259 3592 -2667 ```

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