Updates after upstream drop of legacy LArG4#904
Conversation
Also included an additional example of how that may work. However, the new `photonlibrary_builder_icarus.fcl` is NOT TESTED and PROBABLY WRONG (it uses the handles of the legacy LArG4 to enable its special settings)
The reason why this configuration is not working is not found yet. Possible tracks: physics list does not propagate optical photons; or a module name wrong.
Moved to fcl/archives/LegacyLArG4. These configurations are written for LegacyLArG4, which is now removed, and they need update work to become functional again.
It is broken and old and we should probably just delete it.
These configuration presented a "standard" workflow that includes GEANT4. The update was performed blindly by applying the same pattern as in larg4_icarus.fcl. The only guaranteed success is the test of FHiCL configuration parsing.
There was a problem hiding this comment.
Pull request overview
This PR updates ICARUS job configuration (FHiCL) to accommodate the upstream removal of the legacy LArG4 configuration, with the stated goal of keeping the repository’s FHiCL parsing checks passing.
Changes:
- Introduces
fcl/configurations/larg4_icarus_defs.fcland refactorsfcl/g4/larg4_icarus.fclto consume it (shared “standard” LArG4 services/producers/path). - Updates several PMT simulation/OpReco driver configs to use the “new” LArG4 producer labels (notably
pdfastsim) instead of legacylargeantoptical products. - Removes now-unneeded
largeantmodules*.fclincludes across many configs; deletes the legacyicaruscode/LArG4/largeantmodules_icarus.fcl; adds legacy configs underfcl/archives/LegacyLArG4.
Reviewed changes
Copilot reviewed 48 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| icaruscode/TPC/SignalProcessing/RawDigitFilter/tpcnoiseMC.fcl | Removes obsolete/commented includes to keep config minimal and parsing-clean. |
| icaruscode/TPC/SignalProcessing/RawDigitFilter/tpcnoise.fcl | Same as above for non-MC variant. |
| icaruscode/PMT/trigger_info.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/startingpmtcalibtime.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/prova_source.fcl | Migrates to larg4_icarus_defs.fcl and new LArG4 producer labels/path. |
| icaruscode/PMT/prodsingle_optical_electronic.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/photonlibrary_volumetest_icarus.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/photonlibrary_builder_icarus.fcl | Attempts migration of photon library builder to new LArG4 producer table/path and pdfastsim optical label. |
| icaruscode/PMT/optical_electronic.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/OpReco/driver/study_ophit_singlep.fcl | Switches to larg4_icarus_defs.fcl and new producer labels; updates optical label for mcophit. |
| icaruscode/PMT/OpReco/driver/run_opflash_protons.fcl | Switches to larg4_icarus_defs.fcl; adds loader/pdfastsim producers; updates optical labels. |
| icaruscode/PMT/OpReco/driver/run_opflash_electron.fcl | Same migration pattern as proton config (loader/pdfastsim + label updates). |
| icaruscode/PMT/OpReco/driver/gen_protons.fcl | Switches to larg4_icarus_defs.fcl and adds loader/pdfastsim producers. |
| icaruscode/PMT/OpReco/driver/gen_fakephotons.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/OpReco/driver/gen_fakeflash.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/icarus_prodsingle_fastoptical.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/PMT/icarus_prodsingle_buildopticallibrary.fcl | Deletes old legacy photon library build configuration (superseded by updated builder). |
| icaruscode/PMT/coordinate.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/LArG4/largeantmodules_icarus.fcl | Removes legacy “icarus_largeant” indirection fragment. |
| icaruscode/Filters/filter_particle_active.fcl | Drops legacy largeantmodules.fcl include. |
| icaruscode/Filters/filter_genie_active_interaction.fcl | Drops legacy largeantmodules.fcl include. |
| icaruscode/Analysis/overburden/standard_g4_icarus_21apr21_ob.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/Analysis/overburden/standard_g4_icarus_21apr21_noob.fcl | Drops legacy largeantmodules_icarus.fcl include. |
| icaruscode/Analysis/anaraw_purity_icarus_DQM.fcl | Drops legacy largeantmodules.fcl include. |
| icaruscode/Analysis/anaraw_purity_icarus_DQM_multipletpc.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/overlay/prodoverlay_proton_sbnflux_icarus.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/numi/simulation_numi.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/numi/simulation_numi_numu.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/numi/simulation_numi_nue.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_workshopMar2018.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_workshopMar2018_numucc.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_workshopMar2018_nuecc.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_simple.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_simple_workshop.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_Mar2019.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_Mar2019_nue.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_Mar2019_nue_oscillated.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_Aug2018_numu.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/simulation_genie_icarus_Aug2018_nue.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/prodcorsika_overlay_protononly_icarus.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/prodcorsika_genie_standard_icarus_workshop.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/prodcorsika_genie_standard_icarus_Aug2018_numu.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/prodcorsika_genie_standard_icarus_Aug2018_nue.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/genie/prodcorsika_genie_icarus_Jun2021.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/gen/cosmics/simulation_cosmics_icarus_common.fcl | Drops legacy largeantmodules.fcl include. |
| fcl/g4/larg4_icarus.fcl | Refactors to use larg4_icarus_defs.fcl service/producers/path presets and icarus_all_larg4_services. |
| fcl/configurations/larg4_icarus_defs.fcl | Adds shared ICARUS LArG4 presets: service sets, standard producers table, and standard path sequence. |
| fcl/archives/LegacyLArG4/standard_g4_icarus_21apr21_laronly.fcl | Moves/keeps legacy config in archive area and drops legacy include. |
| fcl/archives/LegacyLArG4/prodsingle_full_optical_electronic.fcl | Adds an archived legacy full optical+electronics workflow configuration. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_numi.fcl | Adds an archived legacy in-time cosmics (NuMI) generation configuration. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_numi_sce_on.fcl | Archived wrapper enabling SCE. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_numi_sce_2d_drift_on.fcl | Archived wrapper enabling 2D drift-only SCE services. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_bnb.fcl | Adds an archived legacy in-time cosmics (BNB) generation configuration. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_bnb_sce_on.fcl | Archived wrapper enabling SCE. |
| fcl/archives/LegacyLArG4/prodcorsika_proton_intime_icarus_bnb_sce_2d_drift_on.fcl | Archived wrapper enabling 2D drift-only SCE services. |
| fcl/archives/LegacyLArG4/dirt_icarus_bnb.fcl | Adds an archived legacy “dirt” simulation configuration. |
| fcl/archives/LegacyLArG4/dirt_icarus_bnb_sce_on.fcl | Archived wrapper enabling SCE. |
| fcl/archives/LegacyLArG4/dirt_icarus_bnb_sce_2d_drift_sim_on.fcl | Archived wrapper enabling 2D drift-only SCE services. |
| fcl/archives/LegacyLArG4/ci_prodcorsika_proton_intime_icarus_bnb.fcl | Archived CI wrapper switching flux copy method to IFDH. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jas1005
left a comment
jas1005
left a comment
There was a problem hiding this comment.
Everything looks good to me, but I'm really only able to give approval at the level of how Gianluca has proposed moving fcl file things around. I don't know anything about the underlying configurations and how they manifest physically. Barring the syntax comment I made and GitHub Copilot's two comments, I'm ready to approve.
cerati
left a comment
cerati
left a comment
There was a problem hiding this comment.
Looks good me, with definitions moved upstream to a new fcl file and the other files updated accordingly. I especially enjoyed Gianluca's bullying Copilot.
|
To be true, a lot of this PR was orchestrated with CoPilot. |
|
✔️ 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 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 |
|
❌ 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 |
|
trigger build LArSoft/larsoft@v10_20_07 LArSoft/lar*@LARSOFT_SUITE_v10_20_07 SBNSoftware/sbnalg@v10_20_07 SBNSoftware/sbnobj@v10_20_07 SBNSoftware/sbnanaobj@v10_20_05 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbncode@v10_20_07 SBNSoftware/icarusutil@v10_15_00 SBNSoftware/icarus_signal_processing@v10_06_00_01 #825 SBNSoftware/icarusalg#94 |
|
✔️ 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 |
The changes proposed here are aimed mainly to pass the FHiCL-parsing unit test.
A half-hearted attempt to correctly update the configuration is sometimes tried, but no test was done except in one case, where the test failed.
List of changes:
larg4_icarus.fclnow refers to definitions inlarg4_icarus_defs.fclinstead of having the same definitions inline. This was tested by verifying that the dumps of the configuration before and after the change are identical.photonlibrary_builder_icarus.fclis a failed attempt to update the photon library creation: the resulting tree is empty (maybe because the physics list is specified in a different way now).fcl/archives/LegacyLArG4as they were, until somebody cares and fixes them.largeantmodules.fcland its ICARUS twin just because it was fashionable. The inclusion has been removed, and they still parse.