-
Notifications
You must be signed in to change notification settings - Fork 1
feat: extract A/E model parameters and compute it in hit.py #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
==========================================
- Coverage 61.58% 61.21% -0.37%
==========================================
Files 16 18 +2
Lines 807 1034 +227
==========================================
+ Hits 497 633 +136
- Misses 310 401 +91 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Well my idea was more stuff would go here
Sent from Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Luigi Pertoldi ***@***.***>
Sent: Saturday, December 20, 2025 8:25:45 PM
To: legend-exp/legend-simflow ***@***.***>
Cc: Dixon, Toby ***@***.***>; Author ***@***.***>
Subject: Re: [legend-exp/legend-simflow] Add a script to extract A/E model pars (PR #37)
⚠ Caution: External sender
@gipert commented on this pull request.
________________________________
On workflow/src/legendsimflow/pars.py<#37 (comment)>:
pars.py is too generic... maybe hpge_pars.py?
—
Reply to this email directly, view it on GitHub<#37 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANSET432EKVB4YC3TJG2O7T4CWWETAVCNFSM6AAAAACPUCMAQGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMBRGI2TMMZRGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@gipert I added some tests, but what I could do is a bit limited with the test-data we have. Eg. it was not really possible to test that both old and new formats will work. |
452eaa1 to
0990fed
Compare
|
No it is not quite right.
*
mean_aoe is not a parameter of the current template function
*
I typically set A_max to 1, to avoid needing to recalibrate A/E later.
Also wouldn't it be better to have a boolean flag like "simulate_psd" or so instead of guessing from the pars being None (for readability).
I'd also move the simulation of all PSD stuff into a function to clean up the script .
Sent from Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Luigi Pertoldi ***@***.***>
Sent: Sunday, December 28, 2025 2:14:30 PM
To: legend-exp/legend-simflow ***@***.***>
Cc: Dixon, Toby ***@***.***>; Mention ***@***.***>
Subject: Re: [legend-exp/legend-simflow] feat: extract A/E model parameters and compute it in hit.py (PR #37)
⚠ Caution: External sender
@gipert commented on this pull request.
________________________________
In workflow/src/legendsimflow/scripts/tier/hit.py<#37 (comment)>:
+ if currmod_pars is not None:
+ # current pulse template domain in ns (step is 1 ns)
+ t_domain = {"low": -1000, "high": 4000, "step": 1}
+
+ # instantiate the template
+ a_tmpl = reboost.hpge.psd.get_current_template(
+ **t_domain,
+ **currmod_pars,
+ )
+ # and calculate A/E
+ a_max = reboost.hpge.psd.maximum_current(
+ chunk.edep_active,
+ drift_time,
+ template=a_tmpl,
+ times=np.arange(t_domain["low"], t_domain["high"]),
+ )
+ aoe = a_max / energy
@tdixon97<https://github.com/tdixon97> is this ok?
—
Reply to this email directly, view it on GitHub<#37 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANSET476B6CSYDSMGRQ7EK34D7QUNAVCNFSM6AAAAACPUCMAQGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMJUG4YDCNBUGY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
ok, now the template max should be at 1. i moved some stuff in a separate function, otherwise i think the hit script looks readable at the moment, we can factor out more in the future if it becomes longer. |
This currently only works on the PRL data, supporting also more recent p14-p16 datasets is possible, but supporting both at the same time is ugly (maybe neccesary for now)