Implementing SimEnergyDeposit Filter for z = 0 in the production release #780
Conversation
…ter around a defined rectangular region representing the unsimulated induction1 wire gap
jzettle
requested review from
SFBayLaser,
acampani,
amenegol,
mvicenzi and
varanini
|
|
||
| icarus_simwire_wirecell_filtersed: @local::icarus_simwire_wirecell | ||
| icarus_simwire_wirecell_filtersed.wcls_main.configs: ["pgrapher/experiment/icarus/wcls-multitpc-sim-drift-simchannel-refactored-filtersed.jsonnet"] | ||
|
|
There was a problem hiding this comment.
I think we either want this to be a new fcl detsimmodules_wirecell_filtered_ICARUS.fcl or the detsim_2d_filter.fcl should happen in the base fcl
There was a problem hiding this comment.
Provided that I understood the logic correctly, I also think it's better to have the fhicl with icarus settings for wire cell separated from that containing the filtering otherwise people might get confused and use the wrong one.
My proposal would be to have
- detsimmodules_wirecell_ICARUS.fcl
- detsimmodules_wirecell_filtered_ICARUS.fcl (that recalls the former and adds what's currently in lines 89-90)
There was a problem hiding this comment.
I think the best thing here is to include this change as part of thedetsim_2d_filter.fcl that way I just change the one wirecell piece there and we don't need this. That should work fine for the variation based approach.
Essentially add:icarus_simwire_wirecell.wcls_main.configs: ["pgrapher/experiment/icarus/wcls-multitpc-sim-drift-simchannel-refactored-filtersed.jsonnet"] to detsim_2d_filter.fcl instead of here
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v09_89_01 SBNSoftware/sbndaq-artdaq-core@v1_09_00of1 SBNSoftware/sbncode#488 SBNSoftware/sbnanaobj@v09_23_00_01 SBNSoftware/sbnobj@v09_19_00_01 SBNSoftware/icarus_signal_processing@v09_88_00_02 SBNSoftware/icarusalg@v09_89_01_01 SBNSoftware/icarusutil@v09_88_00_02 |
|
✔️ 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 |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14: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 e26: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 |
|
I've addressed the comments and for the moment closed the PR to develop so we can focus on this one. We need this one to work in order to make the variations and as I did not make a separate branch for release/SBN2024A and develop I want to make sure this works |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v09_89_01 SBNSoftware/sbndaq-artdaq-core@v1_09_00of1 SBNSoftware/sbncode@v09_89_01_02 SBNSoftware/sbnanaobj@v09_23_00_02 SBNSoftware/sbnobj@v09_19_00_01 SBNSoftware/icarus_signal_processing@v09_88_00_02 SBNSoftware/icarusalg@v09_89_01_01 SBNSoftware/icarusutil@v09_88_00_02 |
|
✔️ 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 |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14: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 |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
A request that this PR is really geared towards a patch release of icaruscode v09_89_01_01p03 for ICARUS systematics. We can include it here for continued functionality in develop, but I need a patch release (github doesn't let me do it or I am not git-savvy enough to have found it quickly). With the other changes here I do not want this currently merged into develop while we figure out more broadly how to include the other pieces of this
Description
Please provide a detailed description of the changes this pull request introduces. If available, also link to a docdb link where the issue/change have been presented on/discussed.
This pull request introduces a larsoft module that can be run during the detsim stage to filter out SimEnergyDeposits around a rectangular volume and creates a new SimEnergyDeposit collection (with a different name, sedfilter) that can be passed to the downstream WireCell processing. The goal of this is to allow for a systematic variation sample that addresses the fact that the induction 1 wire gap at z = 0 is not simulated in the icarus geometry. This PR does not change anything with the standard ICARUS or SBND simulation and the module is completely optional to include.
This PR addresses the fcl configuration and WireCell configuration options of running it as a variation sample and a companion PR in sbncode (SBNSoftware/sbncode#486) addresses the module itself.
Have you added a label? (bug/enhancement/physics etc.)
yes
Have you assigned at least 1 reviewer?
yes
Is this PR related to an open issue / project?
yes, for ICARUS systematics studies for the first oscillation analysis, #769
Does this PR affect CAF data format? If so, please assign a CAF maintainer as additional reviewer.
no
Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)? If so, please link it in the description.
yes, there is a companion PR for icaruscode that includes fcl files and wirecell configuration options in order to run this as a systematic variation
Are you submitting this PR on behalf of someone else who made the code changes? If so, please mention them in the description.
no, this is largely my own work with discussions with @jzennamo