Non-determinism in hit ordering when multiple detectors share readout

[wdconinc]

Check duplicate issues.

• Checked for duplicates

Goal

In the EIC/ePIC simulations we have a far forward detector that consists of four stations. Since the stations are identical and the hits have a high degree of cohesion, we store the hits in the same collection. But since the stations are quite far apart and there is 'stuff' in between them, there are four <detector/> environments in the geometry plugin. (Details: https://github.com/eic/epic/blob/26.06.0/compact/far_forward/offM_tracker.xml)

In the following block:

DD4hep/DDG4/plugins/Geant4DetectorSensitivesConstruction.cpp

Lines 87 to 101 in 7693cca

	for( const auto& iv : p->sensitives ) {
	SensitiveDetector sd = iv.first;
	std::string nam = sd.name();
	auto iter = types.find(nam);
	std::string typ = (iter != types.end()) ? (*iter).second : dflt;
	G4VSensitiveDetector* g4sd = this->createSensitiveDetector(typ, nam);
	for( const TGeoVolume* vol : iv.second ) {
	G4LogicalVolume* g4v = p->g4Volumes[vol];
	if( !g4v ) {
	except("ConstructSDandField: Failed to access G4LogicalVolume for SD %s of type %s.",
	nam.c_str(), typ.c_str());
	}
	ctxt->setSensitiveDetector(g4v, g4sd);
	}
	}

this appears to lead to non-determinism since the four TGeoVolumes for the sensitive volumes are iterated in an order determined by their pointer values. This means that the hits in different planes are not consistently in the same order.

Our goal is for the output of the simulation to be independent of the running conditions (or, in the case of multi-threading, at least the only thing that can vary is the inter-event ordering but not the intra-event ordering inside collections).

Operating System and Version

Debian Trixie

compiler

gcc 14.2.0

ROOT Version

ROOT 6.38.04

DD4hep Version

v01-36

Reproducer

I don't have a minimal reproducer, but the issue is observed in our CI checks. In particular, when we compare the new artifact with the ref artifact we noticed the difference in the ordering of hits (eic/containers#297 (comment)).

$ root -l 'rec_dis_18x275_minQ2=1_epic_craterlake_ref.edm4eic.root' -q -e 'events->Scan("ForwardOffMTrackerHits.cellID","","colsize=20 col=x",1,5)'
Attaching file rec_dis_18x275_minQ2=1_epic_craterlake_ref.edm4eic.root as _file0...
(TFile *) 0x654653b13b20
**********************************************
*    Row   * Instance * ForwardOffMTrackerHi *
**********************************************
*        5 *        0 *                421a0 *
*        5 *        1 *                421a2 *
*        5 *        2 *                    0 *
**********************************************
(long long) 3
$ root -l 'rec_dis_18x275_minQ2=1_epic_craterlake_new.edm4eic.root' -q -e 'events->Scan("ForwardOffMTrackerHits.cellID","","colsize=20 col=x",1,5)'
Attaching file rec_dis_18x275_minQ2=1_epic_craterlake_new.edm4eic.root as _file0...
(TFile *) 0x614ddeba55e0
**********************************************
*    Row   * Instance * ForwardOffMTrackerHi *
**********************************************
*        5 *        0 *                421a2 *
*        5 *        1 *                    0 *
*        5 *        2 *                421a0 *
**********************************************
(long long) 3

Additional context

We do have a way to work around this, but I am (in part) reporting this to document the behavior. I've also stared at this for a while and can't really see a great way to fix it without moving away from a set entirely (and thus potentially breaking interfaces).

[MarkusFrankATcernch]

Hmmmm. Where does this behaviour really come from ?
Normally the the order of the TGeoVolumes should be deterministic unless memory randomization kicks in and
mallocs pointers get randomized rather being monotonically increasing. Since the TGeoVolumes also do not have a unique name within a subdetector, a map using strings would not help either.
If one could properly solve the indexing, this issue can be resolved.

[MarkusFrankATcernch]

@wdconinc
The more I think about this issue, the more I sort of have suspicions about your analysis:

1. The ForwardOffMTrackerHits.cellID should be absolutely independent from the pointer of the TGeoVolume.
2. Geant4 takes one G4Track after the other and tracks it through the volumes one by one. It does not change the order
   of the volumes it tracks the G4Track through. Along this trace for each G4Track the hits are created whenever the
   G4Track moves through a sensitive volume. They are stored in the hits collection one by one as they are created.
   I do not see here a mechanism, which would re-order these hits inside DDG4.
3. From 2) I conclude that a given collection of G4Tracks with the same order should always result in a collection
   of produced hits which necessarily has the same order.

Having said this, if the same set of G4Tracks should result in the same sequence of hits - independent of the order sensitive detectors are assigned to Geant4 volumes. Apparently this is not the case for you.

This leaves me more to the question if somehow the order of the G4Tracks is changed between consecutive events or
something like this. What makes you sure that this assignment of sensitive detectors to Geant4 volumes is the source of this hit re-ordering ? Is there some Geant4 or other feature I am missing ?

[wdconinc]

The more I think about this issue, the more I sort of have suspicions about your analysis:

Fair enough:-) Until one has a fix that demonstrates how to solve this, or a reproducer that doesn't depend on memory ordering this is indeed a valid question with these issues (and others like race conditions).

The conclusion is based on the strong prior that we have never seen non-determinism in any other detector in our entire geometry (we have run this kind of comparison of simulation collections between head and base branches for every commit in every PR in both the simulation geometry and reconstruction framework repository for the better part of three years now). The only detector system and collections this shows up with is in ForwardOffMTrackerHits. If there is an issue at the G4Track level that results in Geant4 registering hits out of order (e.g. due similar set of pointers that are iterated over in https://github.com/Geant4/geant4/blob/41f2dd79968018de4efb966f2546970b30c4af78/source/event/include/G4Event.hh#L275), then it would have been very likely to appear as differences in other collections too. So, this seems to point to this being correlated to either how the DD4hep geometry is created and passed to Geant4 for this particular detector, or how this is handled in the single/multiple G4HCofThisEvent hit collections in Geant4 (I was able to determine though looking at the Geant4 that there is only a single sensitive detector here, and it has multiple G4HCofThisEvent collections that are registered in the order of the G4SDManager::AddNewDetector calls, https://github.com/Geant4/geant4/blob/41f2dd79968018de4efb966f2546970b30c4af78/source/digits_hits/detector/src/G4SDManager.cc#L90).

The second prior is our experience with enforcing determinism in simulations and reconstructions over the past years, both in single-threaded and multi-threaded running. As I mentioned, we do these checks on every commit, so we are by now quite conversant in the types of bugs in contributor code that introduce non-determinism: iterations over containers keyed on pointers, iterations over sorted containers with comparators of pointers (https://github.com/AIDASoft/podio/blob/9bcd402e6e89a1842eead8d0c38883d110771b24/python/templates/Object.cc.jinja2#L48,

DD4hep/DDCore/include/DD4hep/Handle.h

Line 118 in 7693cca

bool operator<(const Handle<T>& element) const {

), combinations of collections that have been filed in different threads where thread join order determines ordering, and then of course the long list of bugs that changes the output beyond ordering (mostly static fields that are not thread localized). So, in this case, that lead to a targeted search in both the DD4hep and Geant4 code bases for set/map of pointer usage with iteration. With the comments above about the ordering of G4SDManager::AddNewDetector calls that register the hit collections, this then resulted in the finding that the set of TGeoVolume iteration results in a non-deterministic ordering of G4SDManager::AddNewDetector calls, a non-deterministic ordering of pointers in the G4SDManager::HCtable and ultimately the G4HCofThisEvents that are returned and stored.

[wdconinc]

The only detector system and collections this shows up with is in ForwardOffMTrackerHits.

Forgot to insert there that the ForwardOffMTracker detector is the only one that has the pattern where we define multiple detectors with the same readout collection. So that also became part of narrowing the targeted search area.

[MarkusFrankATcernch]

if you can reproduce on your desktop try this:
https://docs.oracle.com/en/operating-systems/oracle-linux/9/security/security-ConfiguringandUsingKernelSecurityMechanisms.html

otherwise there are also syscalls to inhibit memory randomization/

cheers,
markus