Skip to content

[NN CF] Momentum vector usage by neural network cluster finder#15582

Draft
ChSonnabend wants to merge 3 commits into
AliceO2Group:devfrom
ChSonnabend:nn_momentum_vector
Draft

[NN CF] Momentum vector usage by neural network cluster finder#15582
ChSonnabend wants to merge 3 commits into
AliceO2Group:devfrom
ChSonnabend:nn_momentum_vector

Conversation

@ChSonnabend

Copy link
Copy Markdown
Collaborator

No description provided.

@ChSonnabend

Copy link
Copy Markdown
Collaborator Author

@davidrohr
Here we can communicate a bit on the implementation of the momentum vector usage created by the cluster finder neural network. This works so far, but I need to make the things nicer and faster still. My main points of discussion:

  • Is the implementation correct in theory / what needs to be adjusted?
  • The current implementation is just work in progress and a bit ugly. Will fix that over the course of the work on it
  • Can we somehow avoid increasing the size of native clusters in case the momentum vector is not used? Is there something like ClassDef that we could use here? Is there a better way to decrease the size? So far I already scaled it to int16_t to reduce disk / memory usage, but maybe there's a smarter way...

Please consider the following formatting changes to AliceO2Group#15582
@davidrohr

Copy link
Copy Markdown
Collaborator

I think for the development and testing, this has to go into a separate class. We cannot allow to increase the size of ClusterNative, if we do not decide that we want to use this in production, which would anyhow only be for Run 4.

@ChSonnabend

Copy link
Copy Markdown
Collaborator Author

I think for the development and testing, this has to go into a separate class. We cannot allow to increase the size of ClusterNative, if we do not decide that we want to use this in production, which would anyhow only be for Run 4.

Ok, thats an easy(ish) fix. Does the rest work?

@davidrohr davidrohr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally that should work, without testing anything or checking in detail

#include <algorithm>
#endif

namespace o2::tpc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why does this have to go here? This is one of the base classes, included almost everywhere, so I would try to keep it as slim as possible.

const float y = y0 + hitData.x * stepY;
const float z = z0 + hitData.y * stepZ;
float nnDydx = 0.f, nnDzdx = 0.f;
const bool useNNDir = tracker.HitNNDirection(row, ih, nnDydx, nnDzdx) && CAMath::Abs(nnDydx) < 10.f && CAMath::Abs(nnDzdx) < 10.f;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

there should be an option in param.rec.tpc, that enables / disables it, and this should be ANDed as first check here.

if (iRow == (int32_t)r.mStartRow + 2) {
tParam.SetSinPhi(dy * ri);
float nnDydx = 0.f, nnDzdx = 0.f;
if (tracker.HitNNDirection(row, seedIH, nnDydx, nnDzdx) && CAMath::Abs(nnDydx) < 10.f && CAMath::Abs(nnDzdx) < 10.f) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't fully understand the math without checking in detail. You are weighting the NN direction and the slope between the clusters? That makes sense. But in any case, as first check, it should be ANDed with a setting in param.rec.tpc

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I normalize the NN direction by the transverse element (ds = sqrt(dx2 + dy2), which becomes dx*invsqrt(1 + (dy/dx)**2)). Since sin(phi) = dy/ds you get nnDydx * nnNormI. The track parameters use the transverse path length with sin(phi) = Py / Pt and DzDs = Pz / Pt. Therefore the normalization is reused for the z direction too.

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.

3 participants