Bug #24834

Trigger TPC offset incorrectly reported by DetectorClocksStandard

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

Target version:
Start date:
Due date:
% Done:


Estimated time:
Occurs In:


The usage pattern:

auto const detClocks = art::ServiceHandle<detinfo::DetectorClocksService>()->DataForJob();
std::cout << detClocks.TriggerOffsetTPC() << std::endl;

seems to return the wrong value.


#1 Updated by Gianluca Petrillo 6 months ago

Same issue for TPC clock period (which can be also observed with the little that is left of DetectorClocksStandard_test:

DetectorClocksStandard_test clockstest_standard.fcl

reports a clock period of 0 microseconds.

#2 Updated by Gianluca Petrillo 6 months ago

  • Occurs In v09_00_00 added

#3 Updated by Gianluca Petrillo 6 months ago

  • Priority changed from High to Normal

Resolution in lardataalg pull request #8.

Two initialisers got swapped, clearly the trigger TPC offset and the TPC frame clock.
Three comments on that.

First, that assignment holds until DetectorClocksServiceStandard::postOpenFile() or DetectorClocksServiceStandard::preBeginRun() is called. This probably means that only initialisations at beginJob() level and non-art initialisations will be affected. For example, I have discovered the issue while figuring out how to properly update gallery@Python code.
So it turns out to be less dramatic than I had thought (probably enough to bust TITUS event display).

Second, I have added back some debugging output method. It has proven to be useful in many occasions in the past, and today would have been as well — it was, in fact, after I restored it. I enabled that output back in the unit test, even if it is effectively a non-automatic test.
If that is against some policy, the test code can be replicated into an utility.

Last, the initialisation code coming with v09_00_00 version of DetectorClocksStandard is more fragile than the previous one: this bug could not have occurred with that older initialisation pattern. I suggest to consider the adoption of a pattern similar to that one, or a different configuration storage, to avoid future issues.

#4 Updated by Lynn Garren 6 months ago

  • % Done changed from 0 to 100
  • Status changed from New to Resolved

lardataalg PR 8 has been merged with develop and will be in the next larsoft release

#5 Updated by Kyle Knoepfel 6 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF