Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: remap RootTreeWriter branches only once #5187

Merged
merged 1 commit into from Jan 15, 2021

Conversation

shahor02
Copy link
Collaborator

The automatic branch substitution mechanism used to store MC labels should not be allowed to
remap branches with >0 entries, otherwise only the last entry will be stored

@davidrohr
Copy link
Collaborator

@shahor02 : I tried this, and now the tpc cluster writer crashes, when I try to store MC labels, while beforehands it did not crash, but beforehand the reader didn't read the labels.

In order to reproduce:

o2-sim -n 10
o2-sim-digitizer-workflow --onlyDet TPC --TPCtriggered
o2-tpc-reco-workflow --infile tpcdigits.root --configKeyValues "GPU_proc.ompThreads=1;GPU_rec.maxTimeBinAboveThresholdIn1000Bin=0;GPU_rec.maxConsecTimeBinAboveThreshold=0;GPU_proc.runQA=1" --output-type tracks

runs through. The QA will output some text at the end Correctly attached clusters: 94953 ( 53.65%) showing that the MC labels are there and OK.

If I run

o2-tpc-reco-workflow --infile tpcdigits.root --configKeyValues "GPU_proc.ompThreads=1;GPU_rec.maxTimeBinAboveThresholdIn1000Bin=0;GPU_rec.maxConsecTimeBinAboveThreshold=0;GPU_proc.runQA=1" --output-type clusters,tracks

with your PR, the cluster writer segfaults.

If I run this same command before your PR, it finishes correctly, and also the QA output reports the correct fraction of attached clusters. However, if I then rerun from a the clusters instead of the digits:

o2-tpc-reco-workflow --infile tpc-native-clusters.root --configKeyValues "GPU_proc.ompThreads=1;GPU_rec.maxTimeBinAboveThresholdIn1000Bin=0;GPU_rec.maxConsecTimeBinAboveThreshold=0;GPU_proc.runQA=1" --input-type clusters --output-type tracks

then there are no MC labels.

Note that the latter works correctly with continuous data, at least before your PR.

The automatic branch substitution mechanism used to store MC labels should not be allowed to
remap branches with >0 entries, otherwise only the last entry will be stored
@shahor02
Copy link
Collaborator Author

@davidrohr could you try now?

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.

Thx, this works now!

@shahor02 shahor02 merged commit 9471e71 into AliceO2Group:dev Jan 15, 2021
@shahor02 shahor02 deleted the pr_remapB branch January 18, 2021 22:45
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.

None yet

3 participants