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

Enable strangeness tracking by default #1598

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

fmazzasc
Copy link
Contributor

This ensures consistency between MC and data reconstruction conditions, where strangeness tracking is enabled by default both in pp and Pb--Pb

Copy link

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2022-pp-apass4
async-2023-pbpb-apass
async-2023-pp-apass1
async-data
async-mc
async-2022-pp-apass6-2023-PbPb-apass2

@fmazzasc
Copy link
Contributor Author

+async-label async-2023-pbpb-apass, async-mc, async-2022-pp-apass6-2023-PbPb-apass2

@benedikt-voelkel
Copy link
Contributor

@fmazzasc What is the runtime with strangeness tracking now? We saw a huge increase back then for PbPb when enabling strangeness tracking.

@fmazzasc
Copy link
Contributor Author

Hi @benedikt-voelkel , in Pb--Pb the strangeness tracking takes ~ 20% of the total SVertexer timing, and it is included in all the reco passes (see for example https://alimonitor.cern.ch/users/download.jsp?view=true&path=/alice/data/2023/LHC23zzo/544961/apass3/0850/o2_ctf_run00544961_orbit0055391072_tf0000767622_epn020/stdout.log).

If you want I can run Pb--Pb a simulation with and without it and tell you which is the overhead

@benedikt-voelkel
Copy link
Contributor

If you could do so, it would be very useful to have a small comparison of the 2 scenarios.
Thanks @fmazzasc !

@fmazzasc
Copy link
Contributor Author

Hi @benedikt-voelkel ,
I tested the SVertexer with and without the strangeness tracking on a MB Pb--Pb simulation (30 events,1 tf, 8 workers).

w/o strangeness tracking: Timing: CPU: 12.08 Real: 3.25 s
w/ strangeness tracking: Timing: CPU: 13.79 Real: 4.36 s

@benedikt-voelkel
Copy link
Contributor

For me, in principle, it is fine. It adds around 33% of time in the vertexing for PbPb and not a factor of ~8 which I think we have seen some months ago.
I do understand that this is somewhat necessary to make MC and data AODs compatible.
@sawenzel Do you agree that we take it and change the default as proposed here?

@sawenzel
Copy link
Contributor

Fine for me. But in principle for anchored MC this setting (could / should) be picked up or transcribed from the reco settings automatically.

@benedikt-voelkel
Copy link
Contributor

Fine for me. But in principle for anchored MC this setting (could / should) be picked up or transcribed from the reco settings automatically.

That would be true if we would extract that in the workflow creation. Currently, we don't do that.
It's not even extracted at the moment: https://github.com/AliceO2Group/O2DPG/blob/master/UTILS/parse-async-WorkflowConfig.py#L147

@benedikt-voelkel benedikt-voelkel added the async-2023-pbpb-apass4 Request porting to async-2023-pbpb-apass4 label Apr 17, 2024
@sawenzel
Copy link
Contributor

Fine for me. But in principle for anchored MC this setting (could / should) be picked up or transcribed from the reco settings automatically.

That would be true if we would extract that in the workflow creation. Currently, we don't do that. It's not even extracted at the moment: https://github.com/AliceO2Group/O2DPG/blob/master/UTILS/parse-async-WorkflowConfig.py#L147

Yes, this is why I was including "should". It would be preferable to have this setting transcribed automatically. For now we can simply merge this PR.

@benedikt-voelkel
Copy link
Contributor

I made the wrong assumption thinking that you mean it might be possible already somehow.
Merging now.

@benedikt-voelkel benedikt-voelkel merged commit 79a2d5c into AliceO2Group:master Apr 17, 2024
7 checks passed
@noferini noferini added async-2023-pbpb-apass3-accepted and removed async-2023-pbpb-apass4 Request porting to async-2023-pbpb-apass4 labels Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants