Support manual addition/exclusion of volumes in HitManager

[sethrj]

This is an attempt to address the multithreading problem where CMSSW defines SDs only on workers but not on the master thread. This prevents the export of the SD tag by the GDML exporter, and it also prevents our HitManager from automatically detecting the LVs that correspond to sensitive volumes.

This MR attempts to replicate the behavior with the skipMasterSD option. However, defining SDs on workers but not master is unfortunately forbidden by the regular Geant4 run manager so we can't reproduce it in-house: the SDs defined on the worker thread are subsequently deleted.

Even though we can't reproduce the exact CMSSW behavior, this MR should provide an alternative path: there is a new SD setup option that lets you define a set of LV pointers which will be later queried for SDs on the worker threads.

Additionally I've added a "skip" option so that we can selectively turn off hit mapping on a per-detector basis, so that we could e.g. have a custom HGCal GPU-based detector but send any other hits back to the CPU.

@drbenmorgan this is the issue I was discussing in today's meeting.

[whokion]

Can you clear compilation errors from HitProcessor.test.cc?

[sethrj]

@whokion I pushed a fix just minutes before you asked ;)

[whokion]

@sethrj I failed to test this PR with CMSSW/cmsRun as there is a crash from VecGeom (unrelated to this MR, see
worker-only-sensdet-cmssw.log with skipMasterSD=false/true, expecting a crash from HitManager with skipMasterSD=false). Testing the standalone app/demo-geant-integration with both this branch and develop, I see a same crash (but somewhat different one from CMSSW), from GeantGeoConverter::convert (see
run3-cms-test.log). So, I guess that we need to address the GeantGeoConverter error first in the develop branch (which I thought that all issues were fixed by @mrguilima). We may merge this now without testing the new feature with CMSSW, but test it later once the Convert related issue is resolved or postpone to merge until then - please advise! @mrguilima, can you confirm that you are able to run demo-geant-integration with cms-run3.gdml or reproduce the crash (you mentioned that some of "!" in volume names were fixed in the gdml. If it is a real issue and related to the crash, can you update the gdml file in the repository?)?

[sethrj]

WTF CI:

/var/jenkins/workspace/Celeritas_PR-772/src/celeritas/user/DetectorSteps.cu -o src/celeritas/CMakeFiles/celeritas.dir/user/DetectorSteps.cu.o
Warning: Function too large, generated debug information may not be accurate.
Error: warning treated as error

[whokion]

@sethrj Is this branch rebased with the remote develop? I want to test this PR with the sd option on as CMSSW+celeritas with the run3 geometry is now working again with all recent updates in VecGeom and celeritas.

[sethrj]

@whokion Yes, it's got all the features that develop has. It would be great if you could test it! I'm writing a unit test now but that can happen asynchronously with your testing. Thanks!

[whokion]

accel/detail/HitManager.cc:144 should follow a similar recipe like if-else blocks in SharedParams.cc as CMS does not use G4RunManager.

[sethrj]

Ooh, good point. I'll fix that right away. Thanks!

[whokion]

Let's merge this for v0.3. Thanks for all good works!

[amandalund]

🐿️

[sethrj]

Thanks! I'll fix it in the v0.3 branch.

[pcanal]

/var/jenkins/workspace/Celeritas_PR-772/src/celeritas/user/DetectorSteps.cu -o src/celeritas/CMakeFiles/celeritas.dir/user/DetectorSteps.cu.o
Warning: Function too large, generated debug information may not be accurate.
Error: warning treated as error

Is the function really large or is it indirectly the case due to in-lining?

[sethrj]

@pcanal I'm guessing it's inlining with debug code that's doing it? The function itself is super short and I was surprised that it's the only one that triggered.