Gather and set more Step/Track attributes when calling sensitive detectors #839

sethrj · 2023-06-28T21:58:26Z

**CHAINED ON #835 **

This introduces a few new options for calling back from GPU to Geant4 sensitive detector classes. With the track option, a particle-type-dependent track will be used for the step, and post-step data will be copied to the track temporarily. A new direction attribute allows the setting of MomentumDirection for pre/post/track.

See #822 .

sethrj · 2023-06-28T22:03:19Z

@whokion I just pushed an update to https://github.com/celeritas-project/cmssw that should (hopefully!) enable all the missing track attributes that caused CMSSW to crash. The track/parent IDs , which are necessary for some of the fine calo work, can't be reconstructed at present because celeritas/geant4 track IDs are completely independent.

whokion · 2023-06-28T22:16:20Z

Okay, will pick up the update and test it with this branch! Thanks!

whokion · 2023-06-29T03:45:05Z

A test result with this update using the CMS HL-LHC geometry hits an exception from SensitiveDetector::cmsTrackInformation(const G4Track* aTrack) :

/data/syjun/cms/externals/src/celeritas/src/accel/detail/HitProcessor.cc:196: debug: Processing 950 hits
%MSG-w SensitiveDetector:  (NoModuleName) 28-Jun-2023 22:18:17 CDT pre-events
 no TrackInformation available for trackID= -1 inside SD EcalHitsEB
%MSG
%MSG-w SimG4CoreApplication:  (NoModuleName) 28-Jun-2023 22:18:17 CDT pre-events
-------- EEEE ------- G4Exception-START -------- EEEE -------
*** G4Exception : sd01
      issued by : SensitiveDetector::cmsTrackInformation()
cannot handle hits without trackinfo
 CMS info: TrackID=-1 ParentID=-1  gamma; Ekin(MeV)=0; time(ns)=0; status=0
   position(mm): (0,0,0); direction: (1,0,0)
   stepNumber=0; stepLength(mm)=0; weight=1 
-------- EEEE -------- G4Exception-END --------- EEEE -------

sethrj · 2023-06-29T11:18:11Z

Crap, I thought that was only needed for fine calo. I'll see if there's some way that I can hack in support for this...

whokion · 2023-06-29T13:27:30Z

Yup, it seems that CMS does weird things for their MC hits, but hope that we are close to handle them anyway - see these places calling cmsTrackInformation, especially from CaloSD and TkAccumulatingSensitiveDetector.

whokion · 2023-06-30T17:46:30Z

Synchronizing CMSSW with the sparse checkout, I reproduce the same exception (that I reported yesterday, but seems not logged in this conversation thread, so re-posting below, note that there is the additional exception from accel/LocalTransporter.cc which may be related to the SD hit issue). I will try to resolve the issue related to SensitiveDetector::cmsTrackInformation() first and may be some others in the down stream.

/data/syjun/cms/externals/src/celeritas/src/accel/detail/HitProcessor.cc:196: debug: Processing 950 hits
%MSG-w SensitiveDetector:  (NoModuleName) 30-Jun-2023 12:30:17 CDT pre-events
 no TrackInformation available for trackID= -1 inside SD EcalHitsEB
%MSG
%MSG-w SimG4CoreApplication:  (NoModuleName) 30-Jun-2023 12:30:17 CDT pre-events

-------- EEEE ------- G4Exception-START -------- EEEE -------
*** G4Exception : sd01
      issued by : SensitiveDetector::cmsTrackInformation()
cannot handle hits without trackinfo
 CMS info: TrackID=-1 ParentID=-1  gamma; Ekin(MeV)=0; time(ns)=0; status=0
   position(mm): (0,0,0); direction: (1,0,0)
   stepNumber=0; stepLength(mm)=0; weight=1 

-------- EEEE -------- G4Exception-END --------- EEEE -------

%MSG
----- Begin Fatal Exception 30-Jun-2023 12:30:17 CDT-----------------------
An exception of category 'Geant4 fatal exception' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'FEVTDEBUGoutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGoutput'
   [3] Calling method for module OscarMTProducer/'g4SimHits'
----- End Fatal Exception -------------------------------------------------
status: Finalizing Celeritas
%MSG-w SimG4CoreApplication:  (NoModuleName) 30-Jun-2023 12:30:17 CDT pre-events

-------- EEEE ------- G4Exception-START -------- EEEE -------
*** G4Exception : celer0005
      issued by : accel/LocalTransporter.cc:183
some offloaded tracks were not flushed
 CMS info: TrackID=-1 ParentID=-1  gamma; Ekin(MeV)=0; time(ns)=0; status=0
   position(mm): (0,0,0); direction: (1,0,0)
   stepNumber=0; stepLength(mm)=0; weight=1 

-------- EEEE -------- G4Exception-END --------- EEEE -------

%MSG

sethrj · 2023-06-30T18:41:51Z

The second error is definitely because of the first: because celeritas aborted mid-step, it didn't finish transporting all the tracks.

When I rebased from 13.0.x onto 13.0.6 I apparently missed a bunch of cmsTrackInfo. I've fixed all those so it's ready for another go.

whokion · 2023-06-30T19:10:47Z

Have you also rollback SimG4CMS/Tracker/src/TkAccumulatingSensitiveDetector.cc ?
I have a compilation error:

/data/syjun/cms/CMSSW_13_0_6/src/SimG4CMS/Tracker/src/TkAccumulatingSensitiveDetector.cc:156:15: error: 'class TrackInformation' has no member named 'setStoreTrack'; did you mean 'storeTrack'?
  156 |         info->setStoreTrack();
      |               ^~~~~~~~~~~~~
      |               storeTrack

and another one:

/data/syjun/cms/CMSSW_13_0_6/src/SimG4CMS/Forward/src/TimingSD.cc:157:15: error: 'class TrackInformation' has no member named 'setStoreTrack'; did you mean 'storeTrack'?
  157 |         info->setStoreTrack();
      |               ^~~~~~~~~~~~~
      |               storeTrack

By pass them, have an except from CaloSD::ProcessHits(G4Step*, G4TouchableHistory*) ()...

whokion · 2023-06-30T23:16:32Z

Okay, the exception is from if-block inside CaloSD - isParameterized is set to true at here inside the HCalSD constructor. So the question is why the HF hit is reached to here for HL-LHC geometry? - again, this is a weird implementation of CMS anyway as HF should use the G4VFastSimulation interface for using cms shower library rather than the ProcessHit interface. Probably should ask Kevin P.

Anyway, by-pass the isParameterized block, cmssw+celeritas with the cms-hllhc geometry configuration runs successfully even though there are some error or warning messages from
accel/detail/HitProcessor.cc:325 (warning: Bumping navigation state by ...)
accel/detail/HitProcessor.cc:346 (warning: Failed to bump navigation state up to ...)
accel/detail/HitProcessor.cc:353 (error: expected step point at ...)

whokion

As long as on-going CMS integration concerns (for Run3 and HL-HLC geometry configurations), the PR is good to go.

sethrj · 2023-07-03T15:54:57Z

@whokion Sorry about the build errors: I'd inadvertently left some of the 13.0.x code in there. That's great news! What was the exception you were seeing in CaloSD::ProcessHits?

I think I should add a JSON output for the 'missed' regions: it's likely that we could see geometry errors (or disagreements between VecGeom and Geant4) if we plot how they're clustered together. We can talk about this Thursday.

whokion · 2023-07-03T16:46:45Z

The remaining exception is from the HF shower library or parameterisation block inside CaloSD.cc, at the line
auto tv = aStep->GetSecondaryInCurrentStep();
in which getting the list of secondary tracks associated with the HF hit and kill them thereafter. Temporarily, I by-pass the block when celeritas offloading is on. Except that, cmsRun+celerias successfully runs with all sensitive detector volumes listed in cms-hllhc-sd-calotk.txt or cms-run3-sd-calotk.txt under the cms benchmar input.

sethrj · 2023-07-03T17:37:31Z

Thanks @whokion , it looks like the step always gets NewSecondaryVector called (whether through G4StepManager or CMSSW TrackingManagerHelper). I'm going to do the same in celeritas, so we should be able to remove the && !useCeleritas in CaloSD::ProcessHits.

whokion · 2023-07-03T18:29:20Z

@sethrj Okay and thanks for the update, which will be tested without the useCeleritas flag. Just to clarify, adding the NewSecondaryVector into the proxy G4Step created by GPU is not necessary if the HF hit processing is implemented in a different way - the fact that it is need for G4SteppingManager or TrackingManagerHelper only matters for the normal Geant4 CPU tracking, not for celeritas tracking on GPU, as they are not used. It is a just side effect to our current hit processing pipeline by the (weird way of CMS) HF hit processing as the hit itself normally should not provide information of the secondary particles created by the current step - the better way of implementing the FAST HF simulation is to use the Geatn4 FastSimulation interface and its own processHit method for the HF shower if either a parameterization method or the shower library is used - i.e., killing the secondary tracks during the hit processing is a weird logic.

sethrj · 2023-07-03T18:41:23Z

Agreed. But it shouldn't hurt to create this new secondary vector (once per simulation) and it's something that Geant4 always does: doing the same should make it less likely to cause a crash in other simulation frameworks or Geant4 applications.

whokion · 2023-07-03T20:12:08Z

Confirm that cmsRun+celeritas runs successfully with updates in celeritas and cmssw along with cms-run3-sd-calotk.txt or cms-hllhc-sd-calotk.txt.

sethrj added enhancement New feature or request external Dependencies and framework-oriented features labels Jun 28, 2023

sethrj added 4 commits June 29, 2023 07:41

Map Celeritas to Geant particles

8714c23

Create track with correct particle type

e6a7d1d

Fix error message

8def98e

Add momentum and track copying to HitProcessor

dd7dffd

Add additional hit process data to track/step

0e96b17

sethrj force-pushed the more-step-data branch from ddbe134 to 0e96b17 Compare June 29, 2023 15:33

whokion approved these changes Jul 3, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into more-step-data

4062f2a

sethrj marked this pull request as ready for review July 3, 2023 16:01

sethrj added 2 commits July 3, 2023 13:17

Create secondary vector when using track data

1972fb3

Always create secondary vector

138160d

sethrj merged commit aae713b into celeritas-project:develop Jul 3, 2023

sethrj deleted the more-step-data branch July 3, 2023 20:36

drbenmorgan mentioned this pull request Jul 31, 2023

Use Acceleritas flag to create G4Track when available celeritas-project/GeoModel#5

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gather and set more Step/Track attributes when calling sensitive detectors #839

Gather and set more Step/Track attributes when calling sensitive detectors #839

sethrj commented Jun 28, 2023

sethrj commented Jun 28, 2023

whokion commented Jun 28, 2023

whokion commented Jun 29, 2023

sethrj commented Jun 29, 2023 •

edited

Loading

whokion commented Jun 29, 2023

whokion commented Jun 30, 2023

sethrj commented Jun 30, 2023

whokion commented Jun 30, 2023 •

edited by sethrj

Loading

whokion commented Jun 30, 2023 •

edited

Loading

whokion left a comment

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

Gather and set more Step/Track attributes when calling sensitive detectors #839

Gather and set more Step/Track attributes when calling sensitive detectors #839

Conversation

sethrj commented Jun 28, 2023

sethrj commented Jun 28, 2023

whokion commented Jun 28, 2023

whokion commented Jun 29, 2023

sethrj commented Jun 29, 2023 • edited Loading

whokion commented Jun 29, 2023

whokion commented Jun 30, 2023

sethrj commented Jun 30, 2023

whokion commented Jun 30, 2023 • edited by sethrj Loading

whokion commented Jun 30, 2023 • edited Loading

whokion left a comment

Choose a reason for hiding this comment

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

sethrj commented Jul 3, 2023

whokion commented Jul 3, 2023

sethrj commented Jun 29, 2023 •

edited

Loading

whokion commented Jun 30, 2023 •

edited by sethrj

Loading

whokion commented Jun 30, 2023 •

edited

Loading