Skip to content

Conversation

@shahor02
Copy link
Collaborator

All integer indices referring to entries of various tracks in their respective containers are
substituted by o2::dataformats::GlobalTrackID, providing both the index and its source container

All int indices referring to entries of various tracks in their respective sources are
substituted by o2::dataformats::GlobalTrackID, providing both the index and its source container
@shahor02 shahor02 requested review from a team, davidrohr and wiechula as code owners January 25, 2021 02:43
Copy link
Collaborator

@davidrohr davidrohr left a comment

Choose a reason for hiding this comment

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

Looks ok to me, it should also work on the GPU eventually as is with minor modifications, if any.

My only comment is that I'd try to use the Global ID sparsely and stick to integers whereever it is clear which container we are accessing. E.g. during TPC ITS matching I would obviously use the integers, in the TrackITSTPC format I am not fully sure (see comment below).

Comment on lines +68 to +69
GlobalTrackID mRefTPC; ///< reference on ITS track entry in its original container
GlobalTrackID mRefITS; ///< reference on TPC track entry in its original container
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not fully sure if it is better to have the GlobalTrackID here directly, or keep the mRefTPC and mRefITS as integer as before, since it is clear where they belong. The conversion to the globalTrackId could also happen in the GlobalTrackID getRefTPC() accessor (which would be just one or operation), there could even be 2 accessors, returning the int for users that know the container, and returning the global ID for general purposes. But OK, not sure if this is overdoing it and whether it would make a difference anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidrohr not really: in the final version the mRefTPC may refer also to TPC track constrained by TOF, TRD or both (GlobalTrackID::TPCTOF, GlobalTrackID::TPCTRD...). Similarly, for the TPCtrack-ITSclusters afterburner I'll need to introduce separate container with matched ITS cluster IDs, this will be yet another GlobalTrackID::Source entry which will be registered in the mRefITS.
For the clear-cut cases I indeed use integers, like the reference on the TOF cluster matched to global track:

evIdx mEvIdxTOFCl; ///< EvIdx for TOF cluster (first: ev index; second: cluster index)
evGIdx mEvIdxTrack; ///< EvIdx for track (first: ev index; second: track global index)

Though, in this particular case there is another problem: TOF still uses for indexing a EvIndex<int, int> to store {tree_entry_ID, index in entry}, so, I've just substituted the reference on the track by EvIndex<int, GlobalTrackID>.
This is a legacy from the times when we did not have just 1 entry per TF (we still have multiple entries for TPC and ITS "triggered" modes, but these trigger implementations are anyway wrong, I would simply introduce extra vector delimiting the ranges of every trigger in a single tree entry). @noferini, I would suppress this tree entry references.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, got it. Initially I was thinking that these constrained tracks of course also have a TPC track ID, but obviously you need the updated constrained track parameters... Makes totally sense as is.

@shahor02 shahor02 merged commit 0c6ec8e into AliceO2Group:dev Jan 25, 2021
@shahor02 shahor02 deleted the pr_gloIdx branch January 25, 2021 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants