restore legacy G4 - Githubissues

cerati commented 6 months ago

This PR is meant to restore the legacy G4 functionality (needs SBNSoftware/icarusalg#84). The main problem is that the legacy G4 does not support multiple sensitive AuxDet volumes corresponding to a single logical volume (see e.g. https://github.com/LArSoft/larsim/blob/develop/larsim/LegacyLArG4/AuxDetReadoutGeometry.cxx#L139) and the refactored gdml uses a single logical volume for all strips in the same module type.

What we do in this PR is to:

ensure that the volume name lookup works for both legacy and refactored gdml (with and without "_" in the volume names). See changes in icaruscode/CRT and icaruscode/CRT/CRTUtils
all the other changes are to fcl files:
- change the default fcls and input labels to the refactored g4, as it should be the new standard
- ensure the legacy g4 and detsim use the legacy gdml and the legacy input labels
- ensure the legacy stage0 fcl for MC use the legacy input label for mcophit (using the new gdml is fine for reco as the geometry is the same)
- comment out mcophit from stage1 fcls as it is not used

What this PR does not do:

move all the legacy fcls to a separate directory
rename the refactored fcls to be the new standard (i.e. what production uses) These should probably be done once there is consensus that we can use refactored for production.

cerati commented 6 months ago

The intended behavior of fcl changes has been verified using @jzennamo's fcl_check.sh script. Thanks Joseph!

jzennamo commented 6 months ago

My reading is that this makes "refactored" closer to the default handling, is that true?

cerati commented 6 months ago

My reading is that this makes "refactored" closer to the default handling, is that true?

Yes. And at the same time revives the legacy

cerati commented 5 months ago

I would have preferred the definition of a complete geometry legacy configuration in geometry_icarus.fcl, instead of
services.Geometry.GDML: "icarus_complete_20220518_overburden.gdml"
services.Geometry.ROOT: "icarus_complete_20220518_overburden.gdml"

Looks like there is already a "legacy" geometry in geometry_icarus.fcl, so to avoid confusion I would not make another one. Also, the refactored is not really a different geometry, it's just a different gdml.

cerati commented 5 months ago

Right, we should have a followup PR with a cleanup of the fcls...

PetrilloAtWork commented 5 months ago

I would have preferred the definition of a complete geometry legacy configuration in geometry_icarus.fcl, instead of
services.Geometry.GDML: "icarus_complete_20220518_overburden.gdml"
services.Geometry.ROOT: "icarus_complete_20220518_overburden.gdml"
Looks like there is already a "legacy" geometry in geometry_icarus.fcl, so to avoid confusion I would not make another one. Also, the refactored is not really a different geometry, it's just a different gdml.

I think it's time to retire that legacy configuration. Also, a new one may have a more descriptive name (geometry_service_GEANTrefactoring?).

cerati commented 5 months ago

I would have preferred the definition of a complete geometry legacy configuration in geometry_icarus.fcl, instead of
services.Geometry.GDML: "icarus_complete_20220518_overburden.gdml"
services.Geometry.ROOT: "icarus_complete_20220518_overburden.gdml"
Looks like there is already a "legacy" geometry in geometry_icarus.fcl, so to avoid confusion I would not make another one. Also, the refactored is not really a different geometry, it's just a different gdml.
I think it's time to retire that legacy configuration. Also, a new one may have a more descriptive name (geometry_service_GEANTrefactoring?).

that can be done, but in a followup PR once we agree what needs to be preserved in fcls in release

mmrosenberg commented 5 months ago

trigger build SBNSoftware/icarusalg#84

FNALbuild commented 5 months ago

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

FNALbuild commented 5 months ago

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

FNALbuild commented 5 months ago

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

:rotating_light: For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

FNALbuild commented 5 months ago

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

:rotating_light: For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

SBNSoftware / icaruscode

restore legacy G4 #716