Bug #24834
Trigger TPC offset incorrectly reported by DetectorClocksStandard
100%
Description
The usage pattern:
auto const detClocks = art::ServiceHandle<detinfo::DetectorClocksService>()->DataForJob();
std::cout << detClocks.TriggerOffsetTPC() << std::endl;
seems to return the wrong value.
History
#1 Updated by Gianluca Petrillo 3 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 3 months ago
- Occurs In v09_00_00 added
#3 Updated by Gianluca Petrillo 3 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 3 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 3 months ago
- Status changed from Resolved to Closed