Skip to content

Add barrel track propagation to PV at AOD creation stage#11458

Merged
sawenzel merged 3 commits into
AliceO2Group:devfrom
nburmaso:pv-preprop
Jun 13, 2023
Merged

Add barrel track propagation to PV at AOD creation stage#11458
sawenzel merged 3 commits into
AliceO2Group:devfrom
nburmaso:pv-preprop

Conversation

@nburmaso

@nburmaso nburmaso commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Dear @ddobrigk, @jgrosseo, @sawenzel

Here is an implementation of (barrel) tracks propagation. It is similar to the track propagation task:

  • tracks with x > 83.1 cm are not propagated
  • if a track has been propagated successfully then it's type is set to aod::track::Track, aod::track::TrackIU otherwise
  • track propagation can be enabled via option --propagate-tracks
  • tracks are propagated using cov matrix if --propagate-tracks-cov is passed (in addition to --propagate-tracks)
  • tracks used by the secondary vertexer are skipped

Please let me know if you have any suggestions

Nazar

@nburmaso nburmaso requested a review from a team as a code owner June 8, 2023 11:52
Comment on lines +472 to +484
void fetchMeanVtxCCDB(int runNumber)
{
static int prevRunNumber = -1;
if (runNumber == prevRunNumber)
return;
auto& ccdbMgr = o2::ccdb::BasicCCDBManager::instance();
std::map<std::string, std::string> headers, metadataRCT;
headers = mCCDBAPI.retrieveHeaders(Form("RCT/Info/RunInformation/%i", runNumber), metadataRCT, -1);
int64_t ts = std::atol(headers["SOR"].c_str()); // timestamp in ms
mVtx = ccdbMgr.getForTimeStamp<o2::dataformats::MeanVertexObject>(mVtxPath, ts);
prevRunNumber = runNumber;
}

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.

Please don't use ccdb manager but use the DPL fetcher, e.g. like:

dataRequest->inputs.emplace_back("meanvtx", "GLO", "MEANVERTEX", 0, Lifetime::Condition, ccdbParamSpec("GLO/Calib/MeanVertex", {}, 1));

pc.inputs().get<o2::dataformats::MeanVertexObject*>("meanvtx");

if (matcher == ConcreteDataMatcher("GLO", "MEANVERTEX", 0)) {

mGIDUsedBySVtx.insert(id3Body.getProngID(0));
mGIDUsedBySVtx.insert(id3Body.getProngID(1));
mGIDUsedBySVtx.insert(id3Body.getProngID(2));
}

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.

Note that now we also going to have the strangeness tracking: fmazzasc@3715b20

const o2::globaltracking::RecoContainer& data,
int colID)
{
if (trackPar.getX() > mMinPropR)

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.

add {}

continue;
}
addToTracksTable(tracksCursor, tracksCovCursor, data.getTrackParam(trackIndex), collisionID);
o2::track::TrackParametrizationWithError<float> trackPar = data.getTrackParam(trackIndex);

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.

here you create a track copy and check for the getX() > Xtpc only in the propagateTrackToPV. It would be faster to 1st check if the track should be propagated then to create its copy

Comment on lines +1629 to +1635
if (mPropTracks) {
mCCDBAPI.init(mCCDBUrl);
auto& ccdbMgr = o2::ccdb::BasicCCDBManager::instance();
ccdbMgr.setURL(mCCDBUrl);
ccdbMgr.setCaching(true);
ccdbMgr.setLocalObjectValidityChecking();
}

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.

don't use the manager, see above.

Comment on lines +2367 to +2383
if (colID >= 0) {
const auto& vtx = data.getPrimaryVertex(colID);
v.setPos({vtx.getXYZ()});
if (mPropTracksCov) {
v.setCov({vtx.getCov()});
}
} else {
v.setPos({mVtx->getXYZ()});
if (mPropTracksCov) {
v.setCov(mVtx->getSigmaX() * mVtx->getSigmaX(),
0.f,
mVtx->getSigmaY() * mVtx->getSigmaY(),
0.f,
0.f,
mVtx->getSigmaZ() * mVtx->getSigmaZ());
}
}

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.

The MeanVertex is in general Z-dependent (the beams have some angles), use just

o2::dataformats::VertexBase v = mVtx.getMeanVertex( colID < 0 ? 0.f : data.getPrimaryVertex(colID).getZ());

mVtx->getSigmaZ() * mVtx->getSigmaZ());
}
}
return o2::base::Propagator::Instance()->propagateToDCABxByBz(v, trackPar, 2.f, mMatCorr, &dcaInfo);

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.

Not sure what is the point of mPropTracksCov option? You are always propagating the cov.matrix.

@nburmaso

nburmaso commented Jun 8, 2023

Copy link
Copy Markdown
Contributor Author

Thank you for the comments @shahor02! I will fix the code and ping you back :)

@nburmaso

nburmaso commented Jun 8, 2023

Copy link
Copy Markdown
Contributor Author

@shahor02, I have tried fixing the code according to your suggestions. Could you please take a look when you have a moment?

@shahor02 shahor02 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.

Thanks @nburmaso
please see 1 comment below, the rest looks fine,

Comment on lines +444 to +446
o2::track::TrackParametrizationWithError<float> trackPar;
bool isProp = false;
if (mPropTracks && trackPar.getX() < mMinPropR) {

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.

Now you have the trackPar undefined before the check :).
I would do:

const auto& trOrig = data.getTrackParam(trackIndex);
bool isProp = false;
if (mPropTracks && trOrig.getX() < mMinPropR) {
  auto trackPar(trOrig);
  if (isProp = propagateTrackToPV(trackPar, data, collisionID)) {
     addToTracksTable(tracksCursor, trackPar, collisionID, aod::track::Track);
  }
}
if (!isProp) {
  addToTracksTable(tracksCursor, tracksCovCursor, trOrig, collisionID, aod::track::TrackIU);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you @shahor02, this looks better indeed :)

@alibuild

alibuild commented Jun 8, 2023

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI for 38887fc at 2023-06-08 16:04:

## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/11458-slc8_x86-64/0/Detectors/AOD/src/AODProducerWorkflowSpec.cxx:447:73: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/11458-slc8_x86-64/0/Detectors/AOD/src/AODProducerWorkflowSpec.cxx:2362:35: error: statement should be inside braces [readability-braces-around-statements]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@shahor02 shahor02 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.

now looks ok to me

@jgrosseo

jgrosseo commented Jun 12, 2023

Copy link
Copy Markdown
Collaborator

Thanks a lot for this work. I have no additional comments.

@sawenzel sawenzel merged commit 01ce2f5 into AliceO2Group:dev Jun 13, 2023
mwinn2 pushed a commit to mwinn2/AliceO2 that referenced this pull request Aug 24, 2023
…p#11458)

* Add barrel track propagation to PV at AOD creation stage
f3sch added a commit to f3sch/AliceO2 that referenced this pull request Dec 7, 2023
This feature was introduced in AliceO2Group#11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
davidrohr pushed a commit that referenced this pull request Dec 8, 2023
This feature was introduced in #11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
chiarazampolli pushed a commit that referenced this pull request Feb 7, 2024
This feature was introduced in #11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
andreasmolander pushed a commit to andreasmolander/AliceO2 that referenced this pull request Apr 12, 2024
This feature was introduced in AliceO2Group#11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
andreasmolander pushed a commit to andreasmolander/AliceO2 that referenced this pull request Apr 12, 2024
This feature was introduced in AliceO2Group#11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
mwinn2 pushed a commit to mwinn2/AliceO2 that referenced this pull request Apr 25, 2024
This feature was introduced in AliceO2Group#11458, however the additional check for SV was
forgotten. This is anyways not enabled by default and should not have affected
any default production.
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.

5 participants