-
Notifications
You must be signed in to change notification settings - Fork 16
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
update the pdhd decoder, now using wibeth #76
Conversation
@emanuele-villa may I merge this myself later today, or do you prefer I wait for one of the reviewers you chose? I don't understand what you changed about the channel map at first glance, but I can use CI to check whether it made any big changes to reconstruction. |
Just saw the other PR that mentions this. I'll try to merge this myself later today. |
trigger build |
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for c14:prof - ignored warnings for build -- details available through the CI dashboard 🚨 For more details about the failed phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
🚨 For more details about the warning phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
trigger build |
I have the suspicion this fcl is not really used since most protodune analyzers have their own. I wanted to mention in dunetrigger an available fcl that could be used to convert raw data to art root, and Hamza had modified this one to work. In particular, now the readout uses WIBeth and the maps (and something) else had to change. This happened before pdune-HD run, that's why I say that I think nobody uses this. I am a bit hesitant to go on without the review, since I might be wrong. |
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
@lhwhitehead @calcuttj if you can take a quick look maybe Andrew can finalize today |
❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for c14:prof - ignored warnings for build -- details available through the CI dashboard 🚨 For more details about the failed phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
🚨 For more details about the warning phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
Let's wait another week for @lhwhitehead and @calcuttj to take a look. |
@emanuele-villa @lhwhitehead @calcuttj any updates this week? I'm going to try to merge PRs before the wine and cheese seminar today. So I'd need this reviewed in about 4 hours from now. |
I don't think we need this update -- there already is a fcl file for decoding the most recent HDF5 files from PDHD run_pdhd_wibeth3_tpc_decoder.fcl we keep the old "outdated" fcls for running the older code for decoding older data, which are written in different formats but still exist on tape. |
e8602a7
to
0d6d8e9
Compare
Following what Tom said, I tested the different decoders and run_pdhd_wibeth_tpc_decoder.fcl worked for recent pdhd data. I therefore removed most of the changes to run_pdhd_tpc_decoder.fcl, I just left the addition of the producer |
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.
Approved
trigger build |
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
trigger build |
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
parent CI build details are available through the CI dashboard |
parent CI build details are available through the CI dashboard |
🚨 For more details about the warning phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
🚨 For more details about the warning phase, check the ci_tests DUNE phase logs parent CI build details are available through the CI dashboard |
This fcl was outdated. @Hamza-Amar had produced a working version using the wibeth options and I just copypasted it to overwrite what we have in the repo.
I'd like this to be updated here since I am mentioning it in the README of dunetrigger, as an example of how to convert raw data to art root. Let me know if there is something else/better that should be used.