Skip to content
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

Major Update to SBND FHiCL Files #663

Merged
merged 50 commits into from
Mar 20, 2025

Conversation

marcodeltutto
Copy link
Member

@marcodeltutto marcodeltutto commented Feb 25, 2025

Description

This PR tries to clean up the simulation fhicls used in SBND. It does the following:

  • Simplifies the naming, eg, for the central value, prodgenie_corsika_proton_rockbox_sbnd.fcl.
  • Restores standard_g4_[...]_sbnd.fcl as the main g4 fhicl
  • Restores standard_detsim_sbnd.fcl as the main detsim fhicl
  • Standard fhicls now use SCE, so the _sce fhicls have been removed or renamed (this applies to both sim and reco configuration files)
  • Removes outdated detsim variation fcls
  • Updates the list of fhicls which are not being tested with the cmake tests (old fhicls have been removed, other have been fixed to pass the test)
  • Most of the _lite files have been removed, and the dropped products have been absorbed in the standard files, since in the end this is what we are running, and having _lite files duplicates the fhicls we need to maintain.

New Standard Workflows

Central value (BNB + Cosmics)
prodgenie_corsika_proton_rockbox_sbnd.fcl
standard_g4_rockbox_sbnd.fcl
standard_detsim_sbnd.fcl

Intrinsic NuE + Cosmics
prodgenie_corsika_proton_intrnue_spill_tpc_sbnd.fcl
standard_g4_sbnd.fcl
standard_detsim_sbnd.fcl

Cosmic Intime:
prodcorsika_proton_intime_sbnd.fcl
standard_g4_intime_sbnd.fcl
standard_detsim_sbnd.fcl

Validation

"Checked" means that fhicl-dump produces the same output.

BNB + Cosmics

Old Name New Name Checked
prodoverlay_corsika_cosmics_proton_genie_rockbox_sce.fcl prodgenie_corsika_proton_rockbox_sbnd.fcl ✔️
g4_sce_dirt_filter_lite.fcl standard_g4_rockbox_sbnd.fcl ✔️
detsim_sbnd_lite.fcl standard_detsim_sbnd.fcl ✔️

Intrinsic NuE + Cosmics

Old Name New Name Checked
prodoverlay_corsika_cosmics_proton_genie_intrnue_spill_tpc_sbnd.fcl prodgenie_corsika_proton_intrnue_spill_tpc_sbnd.fcl ✔️
g4_sce_lite.fcl standard_g4_sbnd.fcl ✔️

In-time Cosmics

Old Name New Name Checked
prodcorsika_proton_intime_filter_sce.fcl prodcorsika_proton_intime_sbnd.fcl ✔️ (SCE is now ON, expected)
g4_sce_simphotontime_filter_lite.fcl standard_g4_intime_sbnd.fcl ✔️

Others

Old Name New Name Checked
prodoverlay_corsika_cosmics_proton_genie_rockbox_fullosc_sce.fcl prodgenie_corsika_proton_rockbox_fullosc_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_genie_rockbox_intrnue_sbnd.fcl prodgenie_corsika_proton_rockbox_intrnue_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_genie_rockbox_sce_keep_corsika_trajectories.fcl prodgenie_corsika_proton_rockbox_keep_corsika_trajectories_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_genie_rockbox_sce_no_shower_rollup.fcl prodgenie_corsika_proton_rockbox_no_shower_rollup_sbnd.fcl -
prodoverlay_corsika_cosmics_cmc_genie_nu_spill_cryostat_sbnd.fcl prodgenie_corsika_cmc_nu_spill_cryostat_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_genie_nu_spill_cryostat_sbnd.fcl prodgenie_corsika_proton_nu_spill_cryostat_sbnd.fcl -
prodoverlay_corsika_cosmics_cmc_genie_nu_spill_tpc_sbnd.fcl prodgenie_corsika_cmc_nu_spill_tpc_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_dirtpropagation_sbnd.fcl prodgibuu_corsika_proton_dirtpropagation_sbnd.fcl -
prodoverlay_corsika_cosmics_proton_genie_nu_spill_tpc_sbnd.fcl prodgenie_corsika_proton_nu_spill_tpc_sbnd.fcl -
prodcorsika_proton_intime_filter.fcl prodcorsika_proton_intime_sbnd.fcl -
prodcorsika_proton_intime_filter_sce_no_shower_rollup.fcl prodcorsika_proton_intime_sce_no_shower_rollup_sbnd.fcl -
g4_sce_simphotontime_filter_no_shower_rollup.fcl g4_intime_no_shower_rollup_sbnd.fcl -
g4_sce_simphotontime_filter_no_shower_rollup_no_mcreco.fcl g4_intime_no_shower_rollup_no_mcreco_sbnd.fcl -
g4_sce_SaveCosmicMCReco.fcl g4_SaveCosmicMCReco_sbnd.fcl -
g4_sce_SaveCosmicMCReco_no_shower_rollup.fcl g4_SaveCosmicMCReco_no_shower_rollup_sbnd.fcl -
g4_sce_dirt_filter_no_shower_rollup.fcl g4_rockbox_no_shower_rollup_sbnd.fcl -
detsim_sce.fcl standard_detsim_sbnd.fcl
prodcorsika_cosmics_cmc_eastwestcrt_mu_filter.fcl prodcorsika_cosmics_cmc_eastwestcrt_mu_filter_sbnd.fcl
prodcorsika_cosmics_cmc.fcl prodcorsika_cosmics_cmc_sbnd.fcl
prodcorsika_cosmics_cmc_filter.fcl prodcorsika_cosmics_cmc_filter_sbnd.fcl
prodcorsika_cosmics_cmc_frontbackcrt_mu_filter.fcl prodcorsika_cosmics_cmc_frontbackcrt_mu_filter_sbnd.fcl
prodcorsika_cosmics_proton_eastwestcrt_mu_filter.fcl prodcorsika_cosmics_proton_eastwestcrt_mu_filter_sbnd.fcl
prodcorsika_cosmics_proton.fcl prodcorsika_cosmics_proton_sbnd.fcl
prodcorsika_cosmics_proton_filter.fcl prodcorsika_cosmics_proton_filter_sbnd.fcl
prodcorsika_cosmics_proton_frontbackcrt_mu_filter.fcl prodcorsika_cosmics_proton_frontbackcrt_mu_filter_sbnd.fcl
prodcorsika_proton_intime_filter_sce_no_shower_rollup.fcl prodcorsika_proton_intime_filter_no_shower_rollup_sbnd.fcl
prodcorsika_proton_intime_sbnd.fcl prodcorsika_proton_intime_sbnd_sbnd.fcl
Other "minor" fhicls are not shown.

Fixes #201, fixes #649, fixes #374, fixes #287.

Checklist

  • Added at least 1 label from available labels.
  • Assigned at least 1 reviewer under Reviewers,
  • Assigned all contributers including yourself under Assignees
  • Linked any relevant issues under Developement
  • Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer. No
  • Does this affect the standard workflow?

Relevant PR links (optional)

Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)?

Link(s) to docdb describing changes (optional)

Is there a docdb describing the issue this solves or the feature added?

@cerati
Copy link
Contributor

cerati commented Feb 25, 2025

Just to say that this is great, and that in ICARUS we should do something that mirrors the result of this PR

@absolution1
Copy link
Contributor

Absolutely smashed it there @marcodeltutto !
Is this ready for review? (220 files!)

@marcodeltutto marcodeltutto added the simulation genie or geant4 (g4) label Feb 25, 2025
@marcodeltutto
Copy link
Member Author

Ready now @absolution1! Yes, I apologize for the many files. I tried to split it in multiple PRs, but if I modify a file that is included in other 100 fcls...well, you get 100 modifies fcls... :(

@marcodeltutto marcodeltutto marked this pull request as ready for review February 25, 2025 23:56
Copy link
Contributor

@absolution1 absolution1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Herculean effort there @marcodeltutto !

I think the vast majority of my comments are about a few fcls that are missing 'sbnd' from the name.

Some of the directories with fcls missing 'sbnd' in their name:

  • crt_filter
  • optical_sim
  • recomb_variations
  • modbox_variations
  • genie/overlay fcls for the flux configs

@marcodeltutto
Copy link
Member Author

marcodeltutto commented Mar 3, 2025

@absolution1 I consolidated the SCE config for sim and reco, and added _sbnd to more files (hopefully all?). There may be some refinements to do later on, but I think out standard workflow should be solid

@marcodeltutto
Copy link
Member Author

I am taking a look at the unit test failures

@bear-is-asleep
Copy link
Contributor

Hi @marcodeltutto , there are a few warnings / errors

CI

The CI needs updated fcl names provided, see these errors for example

61: Testing fcl file prodoverlay_corsika_cosmics_proton_genie_nu_spill_tpc_sbnd.fcl
62: Error parsing prodoverlay_corsika_cosmics_proton_genie_nu_spill_tpc_sbnd.fcl --- Exited with status 90
63: ===================================================================================================================================
64: Failed to parse the configuration file 'prodoverlay_corsika_cosmics_proton_genie_nu_spill_tpc_sbnd.fcl' with exception
65: ---- search_path BEGIN
66:   Can't find file "prodoverlay_corsika_cosmics_proton_genie_nu_spill_tpc_sbnd.fcl"
67: ---- search_path END
68: 
69: ===================================================================================================================================
70: 
71: Testing fcl file prodoverlay_corsika_cosmics_proton_genie_rockbox_sce.fcl
72: Error parsing prodoverlay_corsika_cosmics_proton_genie_rockbox_sce.fcl --- Exited with status 90
73: ===================================================================================================================================
74: Failed to parse the configuration file 'prodoverlay_corsika_cosmics_proton_genie_rockbox_sce.fcl' with exception
75: ---- search_path BEGIN
76:   Can't find file "prodoverlay_corsika_cosmics_proton_genie_rockbox_sce.fcl"
77: ---- search_path END

You should find the relevant files to edit here - https://github.com/SBNSoftware/sbndcode/tree/develop/test/ci

CAF

There are many CAF differences, which could be related to needing to update the workflow. You can see all differences here - https://dbweb8.fnal.gov:8443/LarCI/app/ns:sbnd/storage/docs/2025/03/05/stdout%23wdnX6XB.log

I suppose we need to update the CI before identifying the issues with other tests (including the CAF stage).

@marcodeltutto
Copy link
Member Author

Thanks @bear-is-asleep. I updated the fcl list. That one only checks the fcl integrity though. So the CAF error will persist. I think I know why, I will check and comment back again later

@marcodeltutto
Copy link
Member Author

In the latest CI tests, there are a few warnings:

FHiCL Checks

This is expected. They are related to:

  • EnableSimEfield: this has been eliminated from the FHiCL as the SCE service doesn't have this variable
  • is2DdriftSimHack: this is now set to true at sim, reco, and caf level (was only at sim level before)

G4 and DetSim Stages

This is expected. OpDetBacktrackerRecord and SimPhotonsLite data products are now missing. This is because the _lite version of the fcl files is now incorporated in the "standard" fcls. This is what we have been running in production.

CAF Stage

Differences appear to be on thruth matching variables. This is caused by having is2DdriftSimHack = True. The differences disappear if I run the CI with is2DdriftSimHack = False.

CAFMakes calls the thruth matcher, which uses the SpaceCharge service GetPosOffsets function. The behavior of this function changes based on is2DdriftSimHack. I believe setting it to true is correct, which implies the previous production may not have an accurate truth matching. @jzennamo if you could weigh on this it would be useful! Thank you!

@marcodeltutto
Copy link
Member Author

Regarding the CAF differences, they are all related to stub variables. I have narrowed down the problem to a use of SpaceCharge::GetPosOffsets here which should be GetCalPosOffsets. I'm confirming with Gray, after that, this PR is ready.

@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from marcodeltutto Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@SBNSoftware SBNSoftware deleted a comment from FNALbuild Mar 17, 2025
@bear-is-asleep bear-is-asleep force-pushed the feature/mdeltutt_fhicl_updates branch from 43322df to 762d08f Compare March 19, 2025 07:07
@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@marcodeltutto
Copy link
Member Author

marcodeltutto commented Mar 19, 2025

Summary of CI Warnings

fcl_checks

  • Shows that SCE is now turned ON by default in fcl files. Expected. ✅
  • Shows that simulation fcl files use is2DdriftSimHack: true. Expected. ✅

G4 Stage

  • Shows that sim::OpDetBacktrackerRecord is missing, because lite fcls are now the standard (as they are used in production). Expected. ✅

Detsim Stage

  • Shows that sim::SimPhotonsLite, sim::OpDetBacktrackerRecord and others are missing, because lite fcls are now the standard (as they are used in production). Expected. ✅

CAF stage

  • Small differences observed in shower bestplane and track producer, but those are not reproducible on my end. In any case, it's give more resonable values now compared to defaults in the reference files. ✅

Copy link
Contributor

@jzennamo jzennamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Marco! I look forward to using these!

We should follow-up on the CAF error offline

@bear-is-asleep
Copy link
Contributor

Approved

@bear-is-asleep bear-is-asleep merged commit 2ca3182 into develop Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
simulation genie or geant4 (g4)
Projects
Status: In tagged release
6 participants