Project

General

Profile

Bug #24328

Fix Geometry service feature reloading the geometry at begin of run

Added by Gianluca Petrillo 6 months ago. Updated 20 days ago.

Status:
Work in progress
Priority:
Normal
Category:
Geometry
Target version:
-
Start date:
04/20/2020
Due date:
% Done:

80%

Estimated time:
Occurs In:
Experiment:
-
Co-Assignees:
Duration:

Description

The Geometry service checks at each start of a run which geometry configuration the input data was generated with.
If that geometry is different from the currently configured one, it loads the one matching the input data.

This feature is buggy, broken and dangerous.

It is buggy, because it relies just on the name of the geometry to detect whether a new one is needed. Because it assumes the new geometry file is <GeometryName>.gdml, which is not an advertised requirement. Because it does assume that all the configuration of Geometry service except the detector name and source files are the same. Because it is not able to detect if a new channel mapping algorithm is required by the new geometry.

It is broken, because the requirement of flushing any cached geometry information at run boundaries is not advertised to downstream code.

It is dangerous, because the reload, if completed, may yield erroneous physics results.

This feature has been vocally deprecated. Given the reason above why the feature is broken, I consider it broken beyond repair.
My recommended fix is to leave the code check in geo::Geometry::preBeginRun(), with the understanding that if the test passes it does not guarantee the consistency of geometry (but if it fails it detects a certainly inconsistent one). As response to a failure of this check, the service should just throw a meaningful exception.
In alternative, this feature should be turned off by default and enabled only when explicitly requested by the user, and in that case it should emit information on the console about the feature being enabled, and about each time the geometry is reloaded (right now it does the latter only indirectly).

History

#1 Updated by Kyle Knoepfel 6 months ago

  • Status changed from New to Feedback

We'll contact you to learn more about the context of this request.

#2 Updated by Gianluca Petrillo 6 months ago

  • Assignee set to Gianluca Petrillo
  • Status changed from Feedback to Assigned

#3 Updated by Gianluca Petrillo 20 days ago

  • % Done changed from 0 to 80
  • Status changed from Assigned to Work in progress

The solution discussed with Kyle Knoepfel and Erica Snider is implemented in larcoreobj (pull request #9) and larcore (pull request #6).

Both the operative and technical details of the solution are documented in geo::Geometry class documentation.

In short:
  • GeometryConfigurationWriter, a new mandatory service, writes into each run the geometry configuration once;
  • Geometry service verifies that the geometry configuration of each new run is compatible with the current one.

In addition, provisions to convert existing sumdata::RunData information into something suitable for this check are also in place.

This starts the review phase of the work.



Also available in: Atom PDF