-
Notifications
You must be signed in to change notification settings - Fork 37
CRV calibration scripts #481
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
|
Hi @ehrlich-uva,
which require these tests: build. @Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main. ⌛ The following tests have been triggered for 4847212: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 4847212.
N.B. These results were obtained from a build of this Pull Request at 4847212 after being merged into the base branch at 1e15fc2. For more information, please check the job page here. |
corrodis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these are mainly root macros I don't have any concerns and I think we can merge this in, however there are a few things I don't understand that I just want to bring up for consideration:
- why are the root files openeds in "update" mode? If I run a root macro to write some calibration values to a text file, I would kind of assume the input root files are not modified? (I think they are not but maybe better not open them with "update"?)
- I'm really not in the loop about this, but to first order, to me, it is not clear why the extraction of calibration constants is done in root macros and not in art modules? My, maybe naive, assumption would be that art modules might make it easier to eventually automate the full calibration chain?
|
@corrodis I added you to triage for this repo, could you please re-approve? then I can merge. |
|
The fcl files for creating the pedestal histograms and calibration histograms are regular art jobs with art modules. They run parallel for each subrun to save time. The resulting Root files (with pedestal and calibration histograms from each subrun) are merged ($ROOTSYS/bin/hadd) into a single Root file. The .C scripts use these single Root files to fit the merged histograms. The fit is a simple task that only takes seconds, so I though that a simple Root macro is sufficient. The fit results are added to the histograms. That's why I'm opening the Root files in update mode. If you like, I can just create a new output file. |
This PR does not change simulation and reconstruction.
It requires Offline PR 1644.