Use TOF Utils to subtract BC time to tofSignal #7782

njacazio · 2021-12-01T14:03:53Z

@noferini please have a look

noferini

Ok, from my side.
@chiarazampolli: we couldn't test with aod-producer (we don't know how to do)

jgrosseo · 2021-12-03T07:37:57Z

Detectors/AOD/src/AODProducerWorkflowSpec.cxx

Why has the unit changed by 3 orders of magnitude and the comment stays the same: \mus ?

This is a fix, the time was stored in ns before and the comment was inconsistent. Now we store both trackTime and trackTimeRes with the same units

in the comment you still have mus. Could you write "ns"? I guess this was the comment of @jgrosseo

Sorry, you move to mus everything (I was mislead by the part commented)

Detectors/AOD/src/AODProducerWorkflowSpec.cxx

noferini

I re-generated new AO2D with the last fixes and now everything looks fine.

Let remember to fix in O2Physics this

--- a/Common/Core/PID/PIDTOF.h
+++ b/Common/Core/PID/PIDTOF.h
@@ -227,7 +227,7 @@ class TOFSignal
/// Computes the expected time of a track, given its TOF expected time
/// \param trackTime trackTime in ns
/// \param response response to use to compute the expected time for the propagation from the vertex to TOF
// - static float ComputeTOFSignal(const float& trackTime, const float& expTime) { return trackTime * 1e+3 + expTime; }
// + static float ComputeTOFSignal(const float& trackTime, const float& expTime) { return trackTime * 1e+6 + expTime; }

noferini · 2021-12-03T08:08:08Z

Detectors/AOD/src/AODProducerWorkflowSpec.cxx

Sorry, you move to mus everything (I was mislead by the part commented)

- Use TOF Utils to subtract bunch crossing time - Add PID in tracking into track flags - Clean TOF signal from output object

njacazio · 2021-12-03T08:32:30Z

Thanks @noferini I forgot this, I fixed the comments and am ready to merge!

jgrosseo · 2021-12-03T08:51:23Z

Detectors/AOD/src/AODProducerWorkflowSpec.cxx

-            extraInfoHolder.trackTime = static_cast<float>((tofMatch.getSignal() - exp) * 1e-3 - interactionTime); // tof time in \mus, FIXME: account for time of flight to R TOF
-            extraInfoHolder.trackTimeRes = 200e-3;                                                                 // FIXME: calculate actual resolution (if possible?)
+            extraInfoHolder.trackTime = static_cast<float>((tofSignal - exp) * 1e-3); // tof time in ns, taken from the definition of Ruben scaled by 1000 to convert from mus to ns to match the collisionTime
+            extraInfoHolder.trackTimeRes = 200e-3;                                    // FIXME: calculate actual resolution (if possible?)


@shahor02 I am in favour of merging this. Unrelated: Don't we have an estimate of the track type already somewhere?

@jgrosseo sorry, just saw the question. If you are asking whether we do reco-time PID: no, for that the TPC would need to provide a dedx -> PID algorithm.

@shahor02 I wrote track type but mean track time resolution (sorry for this; and that's why I made the comment here).
So do you already have a track time resolution (even course based on the type of track)?

Not for ITS-TPC-TOF tracks: they are somewhat special since their time is given by the TOF signal stored in the match info (while the kinematic comes from ITS-TPC). In the

AliceO2/DataFormats/Detectors/GlobalTracking/include/DataFormatsGlobalTracking/RecoContainerCreateTracksVariadic.h

Lines 169 to 170 in ce3e745

const float timeErr = 0.010f; // assume 10 ns error FIXME

if (creator(tracksTPCTRD[gidx.getIndex()], {i, GTrackID::TPCTRDTOF}, timeTOFMUS, timeErr)) {

a 10 ns error is assigned (which is surely an overkill but is much more precise that the ITS-TPC time errors), but AOD producer does not use this method, since it relias on the vertex->tracks attachments tables.

I think we should propagate these values into the AOD (even if you mark them as "overkill" for now). All this info should be available in the AOD converter, so it just requires adjusting the code, right?

TrackTime contains here the track time estimate based on TOF (absolute wrt BC). We do not talk about the estimated time with a momentum and particle hypothesis. Do we talk about the same thing?

@jgrosseo for tracks with TRD the error corresponds to the nominal triggered BC duration (i.e. a few 100 ps), for ITS-TPC tracks: uncertainty in matching point Z / vdrift.

Can we add this to the AOD producer already?

I see. A deltat (t_tof - t_track) is avaliable with the matching info.
For the resolution we depend on the track time precision which is available with the track.
Is this what you need?

@jgrosseo

We do not talk about the estimated time with a momentum and particle hypothesis. Do we talk about the same thing?

Does this matter if we speak about the uncertainty?

Can we add this to the AOD producer already?

Do you mean to fill extraInfoHolder.trackTime and extraInfoHolder.trackTimeRes for all tracks types?

Do you mean to fill extraInfoHolder.trackTime and extraInfoHolder.trackTimeRes for all tracks types?

Yes!

njacazio · 2021-12-06T10:43:49Z

@shahor02 if you have no objections I would merge this

njacazio requested a review from a team as a code owner December 1, 2021 14:03

njacazio force-pushed the nj-t0tof branch 2 times, most recently from 705ebe2 to 7f6dc91 Compare December 1, 2021 14:17

noferini previously approved these changes Dec 1, 2021

View reviewed changes

njacazio dismissed noferini’s stale review via 17a2d21 December 1, 2021 17:04

njacazio force-pushed the nj-t0tof branch 2 times, most recently from 17a2d21 to b490e3c Compare December 2, 2021 21:57

njacazio mentioned this pull request Dec 2, 2021

Add first EventTime maker task AliceO2Group/O2Physics#300

Merged

njacazio force-pushed the nj-t0tof branch from b490e3c to e09f20d Compare December 2, 2021 23:26

jgrosseo reviewed Dec 3, 2021

View reviewed changes

Detectors/AOD/src/AODProducerWorkflowSpec.cxx Outdated Show resolved Hide resolved

noferini previously approved these changes Dec 3, 2021

View reviewed changes

Detectors/AOD/src/AODProducerWorkflowSpec.cxx Outdated

Copy link

Collaborator

noferini Dec 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, you move to mus everything (I was mislead by the part commented)

Fix AOD writer workflow

09ea7c4

- Use TOF Utils to subtract bunch crossing time - Add PID in tracking into track flags - Clean TOF signal from output object

njacazio dismissed noferini’s stale review via 09ea7c4 December 3, 2021 08:31

njacazio force-pushed the nj-t0tof branch from e09f20d to 09ea7c4 Compare December 3, 2021 08:31

jgrosseo reviewed Dec 3, 2021

View reviewed changes

jgrosseo approved these changes Dec 3, 2021

View reviewed changes

shahor02 approved these changes Dec 6, 2021

View reviewed changes

shahor02 merged commit ff03ce9 into AliceO2Group:dev Dec 6, 2021

njacazio deleted the nj-t0tof branch December 6, 2021 12:34

	const float timeErr = 0.010f; // assume 10 ns error FIXME
	if (creator(tracksTPCTRD[gidx.getIndex()], {i, GTrackID::TPCTRDTOF}, timeTOFMUS, timeErr)) {

Use TOF Utils to subtract BC time to tofSignal #7782

Use TOF Utils to subtract BC time to tofSignal #7782

Uh oh!

Conversation

njacazio commented Dec 1, 2021

Uh oh!

noferini left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

noferini left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njacazio commented Dec 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgrosseo Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njacazio commented Dec 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

noferini left a comment •

edited

Loading

jgrosseo Dec 7, 2021 •

edited

Loading