-
Notifications
You must be signed in to change notification settings - Fork 1
pedestal leveling #182
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
base: main
Are you sure you want to change the base?
pedestal leveling #182
Conversation
- move level_pedestals task to its own file - move median implementation to pflib::utility - use DecodeAndBuffer already on main
a7871c0
to
e379cdf
Compare
this makes it available across pflib so algorithms can use it
@erik-wallin @taylorjcolaizzi I am requesting your reviews to see how this functions on the HGCROCs you have. I want to make sure that it runs successfully. If you see results similar to mine that is an extra win. I've been testing it by doing a
|
Sure thing! I followed your steps and share my output here. I'm attaching a zip file of my yaml file, since Github won't let me upload .yaml here. level-pedestals-roc-0-settings_20250717_183223.yaml.zip One thing I noticed is that |
And here's a pedestal run I took afterward! |
Thank you both! I now have two more things to look into.
|
[like] Colaizzi, Taylor James (zxk4gs) reacted to your message:
…________________________________
From: Tom Eichlersmith ***@***.***>
Sent: Thursday, July 17, 2025 7:49:24 PM
To: LDMX-Software/pflib ***@***.***>
Cc: Colaizzi, Taylor James (zxk4gs) ***@***.***>; Mention ***@***.***>
Subject: Re: [LDMX-Software/pflib] pedestal leveling (PR #182)
[https://avatars.githubusercontent.com/u/31970302?s=20&v=4]tomeichlersmith left a comment (LDMX-Software/pflib#182)<#182 (comment)>
Thank you both! I now have two more things to look into.
* fix ultra-high TRIM_INV issue. I'm guessing this is because the algo is accidentally finding a negative TRIM_INV which is then being converted to a super-high value when it is static cast into a unsigned int.
* fix upper limit of TRIM_INV. two of the channels that are far below the link median (70 and 34) have a TRIM_INV bigger than 63 mean which is then cropped to only 6 bits and thus are not shifted close to the median at all.
—
Reply to this email directly, view it on GitHub<#182 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BS6CCXVS75437PCXRZ4F4UT3I744JAVCNFSM6AAAAACBSXHRESVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAOBVGI4DAMBSG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
this has been a lurking issue and now that we're trying to write code that algorithmically sets parameters, these checks will be nice
fixed merge conflict in app/tool/tasks.cxx where multiple commands were added at once
Alright, I added some checks in both the compiler and in the level pedestals algorithm. ![]() @taylorjcolaizzi @OwenCole04 @verenamh Please give it a try again, I would appreciate the results in terms of the violin plot that this branch can make.
|
To Do
add another iteration to level even more?going to leave further dev of the algorithm for the future, this is just a first draft and it already works greatDecodeAndBuffer
with one inmain
, move calculation of medians to utilityFunctionality Test
I ran this new task after a hard reset to check how it does. The pftool log of what I did is 180-commands.log
I then ran
to produce the following. The calib channels do not move (as expected, we don't bother leveling them).