Several small generic bug fixes and one typo fix in GatePositroniumSource settings

[wkrzemien]

The fixes are prepared by @MateuszBala:

1. Related to GatePositroniumSource:

• rename setPromptPhotonProbabilites to setPromptPhotonProbabilities (fix typo)

2. Generic fixes (to GATE not related to PositroniumSource):

• event without output
• do not index the hit tree vector after the run cleared it
• keep sourceID when the interaction volume name is empty

Detailed description is given in the first comment.
The command name fix: "setPromptPhotonProbabilities" imposes the changes to the benchmark tests (t35 from the list). The change to benchmarks are being prepared now.

[MateuszBala]

Four independent fixes.

Three are generic - they concern the ROOT output
path, the digitizer and the multi-photon analysis module - and one renames a user-visible command of
GatePositroniumSource, which breaks existing macros.

Each of them is documented separately, with its symptom, its cause, the exact commands to reproduce
it and a before/after measurement, in a repository built for this work:

https://github.com/MateuszBala/opengate-gate-multiphoton-analysis-verification

commit	change	details
8ca6c252	fix(output): do not index the hit tree vector after the run cleared it	bug-07
4aeb6e54	fix(digitizer): keep sourceID when the interaction volume name is empty	bug-02
fbc824fa	fix(multiphoton): skip events without primary vertex quietly	bug-14
548e3460	fix(multiphoton): always run the digitizer at end of event	bug-05
450326fa	fix(source): rename setPromptPhotonProbabilites to setPromptPhotonProbabilities	bug-15
9013f5ea	style(source): correct a typo in the positronium parameters error message	bug-15

1. Every simulation writing ROOT output aborted with libstdc++ assertions

Reported from outside: with -D_GLIBCXX_ASSERTIONS enabled, every simulation writing ROOT
output aborts at the end of the acquisition.

stl_vector.h: std::vector<_Tp, _Alloc>::operator[](size_type)
[with _Tp = GateHitTree*; ...]: Assertion '__n < this->size()' failed.

The consequence is worse than the report suggested: the abort happens before
m_hfile->Write(), so the file is left unwritten. Read with uproot it has not a single key.

configuration	before	after
cylindrical PET with a coincidence sorter	abort, data.root 8.9 MB, 0 keys	no abort, full set of trees (Hits, Coincidences 57 600, pet_data, histograms)
the same scene without any sensitive detector	abort, data.root 264 B, 0 keys	no abort, latest_event_ID, total_nb_primaries, pet_data

In a plain Release build the same read passes silently and returns a stale but still valid pointer,
which is why this had gone unnoticed. It does not depend on the macro - reproduced on three
different configurations.

Cause. GateToRoot::RecordEndOfRun() clears the vectors of trees and buffers.
RecordEndOfAcquisition(), called afterwards, reads m_treesHit[0] to find the file the trees were
written to. Measured with a probe: m_treesHit.size() = 0, capacity = 1. Two reachable ways for
the vector to be empty, both confirmed: the ordinary end of a run, and a configuration with no
sensitive detector at all (BookBeginOfRun iterates over m_SDlist, so it inserts nothing).

Fix. RecordEndOfRun() remembers the file the trees actually went to before clearing the
vector - the last moment at which it can still be done, and it preserves the intent of the original
read, since ROOT splits files above 1.9 GB and the m_hfile from the beginning of the acquisition
may be out of date. RecordEndOfAcquisition() guards the read with !m_treesHit.empty() and
checks that m_hfile exists at all before Write and Close, reporting on G4cerr instead of
dereferencing a null pointer.

The ordinary Release build was checked as well and still writes the full set of trees.

2. The digitizer reset sourceID when the scattering volume name was empty

GateDigitizerInitializationModule::Digitize() modified an unrelated field in reaction to an empty
string:

if ((*inHC)[i]->GetComptonVolumeName().empty()) {
  Digi->SetComptonVolumeName("NULL");
  Digi->SetSourceID(-1);            // side effect of an empty name
}
if ((*inHC)[i]->GetRayleighVolumeName().empty()) {
  Digi->SetRayleighVolumeName("NULL");
  Digi->SetSourceID(-1);
}

The same code exists in the twin of that method, GateHitConvertor::ProcessOneHit(), used by the
older digitizer chain. The -1 is not a sentinel anyone reads: no place in the code compares
sourceID with -1, and GateToLMF casts the field to unsigned short, where -1 becomes
65 535.

How it surfaced: with GateMultiPhotonAnalysis enabled, every single and every coincidence carried
sourceID = -1, while the same macro with GateAnalysis reported the correct source index.

field, multianalysis	before	after
Singles.sourceID	-1 for 179 422 / 179 422	0 for 179 422 / 179 422
Coincidences.sourceID1, sourceID2	-1	0 for 57 600 / 57 600

The same macro with GateAnalysis reported sourceID = 0 for all 179 422 singles both before and
after the change.

GateAnalysis and GateFastAnalysis always write a non-empty name into the hit - the first by
initialising its variables with the literal "NULL", the second by an explicit
SetComptonVolumeName("NULL") - so for them the branch was never taken and nothing changes.
GateMultiPhotonAnalysis leaves the name empty, so for that module the branch was reached on every
hit and the field was destroyed. The empty name itself is a defect of that module and is not
fixed here; it is part of the multi-photon analysis work that will follow. This commit removes the
coupling, which is what makes sourceID correct again regardless of which module filled the hit.

Fix. The two SetSourceID(-1) calls are removed from both places. An empty name is still
replaced by "NULL". In addition, GateHit::GateHit() now initialises m_sourceID to -1, so
that "nobody filled this field" is a state established where the hit is created rather than one
attached conditionally by the digitizer.

3. GateMultiPhotonAnalysis: a false warning, and an event with no detector response

Two defects in the same branch of RecordEndOfEvent.

Every simulation using multianalysis ended with one G4Exception warning:

*** G4Exception : GateMultiPhotonAnalysisMissingTrajectoryContainer
      issued by : GateMultiPhotonAnalysis::RecordEndOfEvent
GateMultiPhotonAnalysis missing trajectory container for run=0, event=99142.
hasProcessableHits=false. Policy=resilient.

Always one event per run, always with hasProcessableHits=false. The event number matches the
counters written into the ROOT file exactly (total_nb_primaries = 99142,
latest_event_ID = 99142): it is the last event of the run. GateSourceMgr::PrepareNextEvent
generates no vertex once the time limit is exceeded, Geant4 still processes the event to the end,
but there is not a single track in it - and G4Event::GetTrajectoryContainer() returns a container
only once the first trajectory has been stored. Under the strict policy the same event would have
aborted an otherwise correct simulation.

Fixed by ending the processing of an event with GetNumberOfPrimaryVertex() == 0 quietly: no
G4Exception, no increment of the anomaly counters. The condition is strictly equivalent to the
description of the case - no primary vertex means no track, hence neither hits nor a trajectory
container. The warning and the counters are left untouched for the case that really is an anomaly:
an event with hits but without trajectories.

build	MissingTrajectoryContainer warnings per run	data
before	1	317 154 hits
after	0	317 154 hits, columns identical row by row

An event abandoned by the analysis produced no singles and no coincidences at all.
RecordEndOfEvent returned early when the event had no trajectory container, and the call to the
digitizer stood at the end of the method, so the hits were there but nothing turned them into a
detector response. GateAnalysis places the same call outside its if (!trajectoryContainer)
block, so it digitises regardless.

Fixed with a scope guard in an anonymous namespace, ScopedDigitizerRunner, whose destructor calls
RunDigitizersIfNeeded(); the explicit call at the end of the method is removed, so the digitizer
runs no matter which way the analysis of the event ends. Writing the call before each return was
rejected: there are three of them today and there may be a fourth tomorrow. The guard is created
after the tracking-mode check, not before it - an unsupported tracking mode raises a
FatalException whose message says explicitly that further processing, digitisation included, is
skipped, and that path is left as it was.

The situation does not arise on its own, since GATE stores trajectories for all tracks, so it was
produced deliberately: two binaries differing only in this fix, both with
trajectoryContainer = nullptr; inserted right after the container is read, running the same short
PET simulation with Singles enabled.

build	Hits	Singles
before	5 992	0
after	5 992	4 228

No regression in ordinary running: the same simulation gives the same 305 207 hits and 216 312
singles as before.

4. setPromptPhotonProbabilites renamed - breaking change

The command configuring the emission probability of the prompt gamma was missing the i of
"Probabilities", while the neighbouring command of the same messenger is spelled correctly
(setElectronCaptureProbabilities). A user writing the name the natural way got
***** COMMAND NOT FOUND *****. The C++ API was already correct - the messenger calls
fParamGenerator.SetPromptGammaProbabilities(...) - so the typo lived purely in the user
interface. grep over source/ gives three places: the registration string, the comparison in
SetNewValue and the name of the member; the documentation repeated the misspelling three times.

No alias is registered. A macro using the old name does not merely warn on the new build, it
aborts the simulation and produces no ROOT file at all:

[G4-cerr] ***** COMMAND NOT FOUND </gate/source/testSource/setPromptPhotonProbabilites 1.0> *****
[Gate] Sorry, error in a macro command : abort.

Registering the old name as a deprecated alias with a warning is a two-line addition if the
collaboration prefers that; as submitted, this needs a line in the release notes and an update of
every macro using the old spelling, the tests included.

A second, purely cosmetic commit corrects the same misspelling in the text of an error message in
GatePositroniumDecayParamsGenerator.cc ("Electron Capture Probabilites"). That is message text
rather than a command name, hence a separate commit.

Verification

The repository linked above builds two variants of GATE - the commit this work started from and the
branch with the fixes - runs the same macros against both and compares the resulting trees. For the
four fixes in this pull request the criteria are measurements rather than unit tests, because three
of them concern situations a macro cannot express:

fix	how it was checked
ROOT output	build with -D_GLIBCXX_ASSERTIONS, run the scene, list the keys of data.root. In a plain Release build the defect is invisible, so the assertion build is essential
sourceID	the sourceID column of the Singles and Coincidences trees, both analysis modules, same macro
false warning	the content of run.log, plus a row-by-row comparison of the Hits tree before and after
digitizer skipped	two binaries differing only in this fix, with the anomaly forced by a one-line patch
command rename	all ten positronium simulations recomputed in both analysis variants after the rename; no COMMAND NOT FOUND, every one with a data.root

git clone https://github.com/MateuszBala/opengate-gate-multiphoton-analysis-verification.git
cd opengate-gate-multiphoton-analysis-verification
make env
cp external/scripts/environment.sh.template .environment.sh   # set GEANT4 and ROOT
make clone-gate
make build-ref-develop && make build-ref-develop-assert
make build-ref-fix    && make build-ref-fix-assert
make run-digitizer-scene-assert     SIM_DIR=simulations/digitizer/reconstruction   # aborts
make run-digitizer-scene-fix-assert SIM_DIR=simulations/digitizer/reconstruction   # passes
make run-set SET_DIR=simulations/ref-fix/gate-multi-photon-analysis
make tests

Not in this pull request

The four fixes above are the part of the work that is independent of the multi-photon analysis
itself. Still to come, in separate pull requests: the interaction counting of
GateMultiPhotonAnalysis (the defect this work started from), the septal penetration counter, the
continuity of the counters across hit collections, the propagation of the decay fields to the
Singles and Coincidences trees, the coincidence sorter under takeWinnerIfOnlyOneGood, and an
attachCrystalSD request that is reported as ignored while the sensitive detector has already been
registered. Each is described in
docs/bugs.

Two items are documented there and deliberately left out entirely: initialising the remaining
plain-data members of GateHit (preventive rather than corrective, and it touches every sensitive
detector and every output module), and a segmentation violation in
G4RunManager::DeleteUserInitializations() during application shutdown, which reproduces
identically on the commit this work started from - the data is already written and closed when it
happens.