-
Notifications
You must be signed in to change notification settings - Fork 40
Removed flash filter from Stage1 standard configuration #577
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
Removed flash filter from Stage1 standard configuration #577
Conversation
Configuration of that ilter throughout the repository has been fixed.
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v09_73_00 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c7:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e20:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v09_73_00 SBNSoftware/sbncode@v09_73_00 SBNSoftware/sbndaq-artdaq-core@v1_06_00of0 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
SFBayLaser
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.
Is there a reason to not also delete the file "stage1_run2_nofilter_icarus.fcl" since it will be redundant? Or am I just not seeing that in the list?
Otherwise, thank you for addressing this as it was obvious this was quite out of date.
|
No strong reason. I did not remove it because I thought it may have a role if the CRT/PMT matching-based filter is introduced, but then it will probably be at Stage0 anyway. |
They are equivalent to the mainline configurations, by which they are now replaced.
|
Adopted the reviewer suggestion of removing the configurations names |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v09_73_00 SBNSoftware/sbncode@v09_73_00 SBNSoftware/sbndaq-artdaq-core@v1_06_00of0 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase unit_test ICARUS on slf7 for e20:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the unit_test ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase unit_test ICARUS on slf7 for c7:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the unit_test ICARUS phase logs parent CI build details are available through the CI dashboard |
|
The test is right, a fix is needed. |
|
Please run the test again: it should be fixed now. |
|
trigger build |
|
✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for c7:prof - ignored warnings for build -- details available through the CI dashboard 🚨 For more details about the failed phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
|
trigger build |
|
✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
The job configuration
stage1_run2_icarus.fclfeatured a flash filtering which has been outfashioned by the presence of a trigger and will be obsoleted by filtering based on CRT/PMT matching.The filter in that job is also misconfigured, using a very old time reference for its flash window specification which is now way out of the PMT readout range. That means that using that configuration will suppress all the input events.
This pull request:
Stage1configurations (Run1 and Run2); effectively,stage1_run2_icarus.fclis now equivalent tostage1_run2_nofilter_icarus.fcl;Stage0definition (which is not used anyway)I took the chance to also update the list of configurations that should be skipped by the FHiCL syntax test.
I will follow with a pull request for the production branch.
Suggested reviewer: