Skip to content

FIX: Reading calibrated timing in MModuleLoaderMeasurementsROA#120

Merged
ckierans merged 1 commit into
cositools:develop/emfrom
fhagemann:roa
Mar 31, 2026
Merged

FIX: Reading calibrated timing in MModuleLoaderMeasurementsROA#120
ckierans merged 1 commit into
cositools:develop/emfrom
fhagemann:roa

Conversation

@fhagemann

Copy link
Copy Markdown

The ROA loader is currently assigning the values for Timing to m_TAC instead of m_Timing.
This is fixed now.

@fhagemann fhagemann added the bug Something isn't working label Mar 25, 2026
@fhagemann fhagemann changed the title Bug fix in MModuleLoaderMeasurementsROA FIX: Reading calibrated timing in MModuleLoaderMeasurementsROA Mar 25, 2026
@fhagemann

fhagemann commented Mar 26, 2026

Copy link
Copy Markdown
Author

Also: what is m_Timing supposed to be? The calibrated TAC value in nanoseconds? Or the drift time since the creation of electron hole pairs (in nanoseconds)?

According to the comment in MStripHit, it should be a double denoting the //! Timing in ns

//! Timing in ns
double m_Timing;

In the ReadOutAssemblyTiming in megalib, m_Timing is an unsigned int denoted as //! The timing:
https://github.com/zoglauer/megalib/blob/c5fa4af354fc36be4cd71511402a68f0d6d6952f/src/fretalon/base/inc/MReadOutDataTiming.h#L90-L91

Currently, this would write floating point numbers to the file, but expects integers when reading them, resulting in very funky behavior.

@ckierans

Copy link
Copy Markdown
Collaborator

Good catch!

m_Timing should be the TAC calibrated into ns. m_TAC was a variable Sean introduced awhile back. We used to only have the Timing keyword and it was overloaded to represent the ADC value and the ns conversion. m_Timing should be a floating point whereas m_TAC should be an int.

@ckierans ckierans merged commit fc79014 into cositools:develop/em Mar 31, 2026
@fhagemann

Copy link
Copy Markdown
Author

But changing the type of m_Timing this something that we need to tackle in megalib (on the develop-cosi branch)
then? If so, I can open an issue / draft a PR for that.

@fhagemann fhagemann deleted the roa branch March 31, 2026 16:36
@zoglauer

Copy link
Copy Markdown
Collaborator

The MEGalib part is what we read from the roa file, the TAC value.
Thus is should be an integer

@fhagemann

fhagemann commented Mar 31, 2026

Copy link
Copy Markdown
Author

The MEGalib part is what we read from the roa file, the TAC value. Thus is should be an integer

But let's assume we write calibrated timing values to ROA, what would be the preferred reader then?
I would then need to use ReadOutDataTiming for the m_TAC and something else (that would then still need to be written?) for m_Timing.
I feel the naming is a bit inconsistent here..

@zoglauer

Copy link
Copy Markdown
Collaborator

Yes, Timing is from the old days. We didn't call it TAC in the balloon days. But why do we write ns timing to roa files? roa files are for the data from the raw measurements.

@fhagemann

Copy link
Copy Markdown
Author

Well, the current ROA loader has to possibility to read "TAC" and "Timing" and attributes it to the m_TAC and m_Timing value of the strip hits (?)

No my question would be if this way of reading the data is actually correct or not?

for (unsigned int r = 0; r < Event->GetNumberOfReadOuts(); ++r) {
MReadOut RO = Event->GetReadOut(r);
const MReadOutElementDoubleStrip* Strip =
dynamic_cast<const MReadOutElementDoubleStrip*>(&(RO.GetReadOutElement()));
const MReadOutDataADCValue* ADC =
dynamic_cast<const MReadOutDataADCValue*>(RO.GetReadOutData().Get(MReadOutDataADCValue::m_TypeID));
const MReadOutDataTiming* Timing =
dynamic_cast<const MReadOutDataTiming*>(RO.GetReadOutData().Get(MReadOutDataTiming::m_TypeID));
const MReadOutDataTAC* TAC =
dynamic_cast<const MReadOutDataTAC*>(RO.GetReadOutData().Get(MReadOutDataTAC::m_TypeID));
const MReadOutDataEnergy* Energy =
dynamic_cast<const MReadOutDataEnergy*>(RO.GetReadOutData().Get(MReadOutDataEnergy::m_TypeID));
const MReadOutDataOrigins* Origins =
dynamic_cast<const MReadOutDataOrigins*>(RO.GetReadOutData().Get(MReadOutDataOrigins::m_TypeID));
MStripHit* SH = new MStripHit();
SH->SetDetectorID(Strip->GetDetectorID());
SH->IsXStrip(Strip->IsLowVoltageStrip());
SH->SetStripID(Strip->GetStripID());
if (Timing != nullptr) {
SH->SetTiming(Timing->GetTiming());
}
if (TAC != nullptr) {
SH->SetTAC(TAC->GetTAC());
}
if (Energy != nullptr) {
SH->SetEnergy(Energy->GetEnergy());
}
if (ADC != nullptr) {
SH->SetADCUnits(ADC->GetADCValue());
}
if (Origins != nullptr) {
SH->AddOrigins(Origins->GetOrigins());
}
Event->AddStripHit(SH);
}
Event->SetTimeUTC(Event->GetTime());
return true;
}

@fhagemann

Copy link
Copy Markdown
Author

And there is also possibility to read/write ADC and Energy, the equivalent for the slow shaper output.

@fhagemann

Copy link
Copy Markdown
Author

This is a screenshot of what I have:
adc = (Uncalibrated) energies in ADC units
tac = (Uncalibrated) TAC values in ADC units
energy = (Calibrated) energies in keV
timing = (Calibrated) TAC values in ns

Note that it's the same quantities, just differing in calibration/units.
Timing right now would NOT be drift time or something.
It's still the TAC value, just in nanoseconds.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants