AliceO2 icon indicating copy to clipboard operation
AliceO2 copied to clipboard

Add EP angle to McCollisions for debugging

Open ddobrigk opened this issue 1 year ago • 2 comments

This PR adds the event plane (reaction plane -> the angle of inclination of the impact parameter from PYTHIA) to the McCollisions table. It is necessary as a development for the v2 MC closure testing. Note that, if the data model change is accepted, a converter will be written and committed to O2Physics before the McCollisions table version is actually bumped to 001.

The current size of the McCollisions table in our Pb-Pb MC is 0.002% of all compressed space, and it is comprised of 6 floats and one short. Adding one float to it will result in negligible disk space use that is well worth the cost. I would be happy to present the case for this in one of the WP4+WP14 meetings, of course. Thank you!

Tagging @sawenzel @pzhristov @ktf

ddobrigk avatar Jul 10 '24 08:07 ddobrigk

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-2023-pbpb-apass3 async-2023-pbpb-apass4 async-2023-pp-apass4 async-2024-pp-apass1 async-2022-pp-apass7 async-2024-pp-cpass0

github-actions[bot] avatar Jul 10 '24 08:07 github-actions[bot]

You can find more information on this effort here: https://www.dropbox.com/scl/fi/ss57me45ykp2f6960a2gt/DDChinellato-SynthV2Gen-01.pdf?rlkey=pph4d1286giy3sd65rz136010&dl=0

ddobrigk avatar Jul 10 '24 08:07 ddobrigk

Looks good to me. Should probably be merged only when the missing pieces have been filled (once AOD format is modified). Maybe it makes sense to add "draft" until this is done.

sawenzel avatar Jul 10 '24 12:07 sawenzel

Looks good to me. Should probably be merged only when the missing pieces have been filled (once AOD format is modified). Maybe it makes sense to add "draft" until this is done.

@sawenzel actually, I would say we should merge this now, because we need to have the converter in O2Physics before this can proceed. I am then unsure about what is actually missing - the files that are touched by this PR actually include (commented out) all necessary changes and, if I uncomment them and use McCollisions_001, I immediately get PYTHIA event plane angles saved in AO2Ds - it already works. The only reason why these other changes are commented out is that, unless we merge this now and write a converter, we will break all existing AO2Ds :-)

ddobrigk avatar Jul 10 '24 13:07 ddobrigk

Looks good to me. Should probably be merged only when the missing pieces have been filled (once AOD format is modified). Maybe it makes sense to add "draft" until this is done.

@sawenzel actually, I would say we should merge this now, because we need to have the converter in O2Physics before this can proceed. I am then unsure about what is actually missing - the files that are touched by this PR actually include (commented out) all necessary changes and, if I uncomment them and use McCollisions_001, I immediately get PYTHIA event plane angles saved in AO2Ds - it already works. The only reason why these other changes are commented out is that, unless we merge this now and write a converter, we will break all existing AO2Ds :-)

Ok fine. But I don't understand a few addition changes like /*, 0.0*/ which seem to add nothing to the PR. I understood them as preparing for future changes. I also don't see where the eventPlane angle is actually written to AOD. Could you please highlight the relevant line for my understanding?

sawenzel avatar Jul 11 '24 07:07 sawenzel

Hi Sandro, so let me try to explain: the two changes:

  • /*, 0.0*/ and the other relevant change,
  • /*, getEventInfo(header, Key::planeAngle, header.GetRotZ())*/,

are exactly what makes the whole thing work once we switch to the new version of the McCollisions table. The point is that this PR actually creates McCollisions_001, but does not switch to using it as default: we still declare using McCollisions = McCollisions_000;, so the cursor filling that happens in the two commented parts above should not be changed yet. However, once we merge this PR, then we can write a converter in O2Physics that creates McCollisions_001 from McCollisions_000 by inserting a dummy value of the event plane angle in the newly created table. This converter will make it possible to still use existing MC AO2Ds without troubles. Once the converter is in place, the next PR will switch to using McCollisions = McCollisions_001; and uncomment the two comments above. The first one, in the standalone AO2D producer, is necessary or the cursor filling will not compile; the second one, in the regular AO2D producer, is exactly where, in some sense, we are ensuring that the event plane will get saved in the AO2Ds because it will then be part of the McCollisions(_001) table.

If you feel that having these comments in the meantime is unnecessary, I have absolutely no problem with removing them for the time being - either way, the moment we switch to McCollisions_001, those are the changes that will come. Thanks!

ddobrigk avatar Jul 11 '24 08:07 ddobrigk

No it's fine. In the end it is as I presumed.

sawenzel avatar Jul 11 '24 08:07 sawenzel

+async-2023-pbpb-apass4

ddobrigk avatar Nov 22 '24 11:11 ddobrigk

+async-label async-2023-pbpb-apass4

ddobrigk avatar Dec 03 '24 09:12 ddobrigk