AliPhysics icon indicating copy to clipboard operation
AliPhysics copied to clipboard

PWGEM:LMee: Adding new KFParticle variables to TreeMaker

Open jjungIKF opened this issue 1 year ago • 11 comments

jjungIKF avatar Jul 02 '23 08:07 jjungIKF

e9024539b0433906da2e4006d5ad2d48be6ff94b: approval required: 1 of @atoia (Alberica Toia), @chiarazampolli (Chiara Zampolli), @davidrohr (David Rohr), @dsekihat (Daiki Sekihata), @hmurakam (Hikari Murakami), @iarsene (Ionut Cristian Arsene), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @otonvd (Oton Vazquez Doce), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @rbailhac (Raphaelle Bailhache), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan), @tkgunji (Taku Gunji), @wiechula (Jens Wiechula)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

alibuild avatar Jul 02 '23 08:07 alibuild

+1

dsekihat avatar Jul 02 '23 10:07 dsekihat

e9024539b0433906da2e4006d5ad2d48be6ff94b: approved: will be automatically merged on successful tests

alibuild avatar Jul 02 '23 10:07 alibuild

Error while checking build/AliPhysics/root6 for e9024539b0433906da2e4006d5ad2d48be6ff94b at 2023-07-05 18:28:

## sw/BUILD/AliRoot-latest/log
100% tests passed, 0 tests failed out of 101


## sw/BUILD/AliPhysics-latest/log
115/116 Test  #68: load_library_PWGDQdielectron .....................***Failed   38.92 sec
116/116 Test  #70: load_library_PWGDQreducedTree ....................***Failed   38.80 sec
98% tests passed, 2 tests failed out of 116

Full log here.

alibuild avatar Jul 02 '23 20:07 alibuild

@TimoWilken Hi Timo, sorry for pinging you. You commented earlier on my old pull request. Unfortunately, I cannot open the link you provided in your post anymore. Could you help me again with this issue? I am currently a bit puzzled as I compile it locally and the tests run without issues.

jjungIKF avatar Jul 04 '23 15:07 jjungIKF

Hi @jjungIKF!

I was referring to the following lines you added after the new if(KFParticle_FOUND) block in PWGDQ/dielectron/CMakeLists.txt:

set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES})
generate_rootmap("${MODULE}" "${LIBDEPS}" "${CMAKE_CURRENT_SOURCE_DIR}/${MODULE}LinkDef.h")

In your previous PR, I think you didn't delete the same lines further above (lines 136+137), but now it seems you have.

I'm still a bit confused why you've done this -- doesn't the set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES}) after the set(LIBDEPS ${LIBDEPS} ${KFPARTICLE_LIBRARY}) wipe that change out again?

Also, in this case, presumably ${KFPARTICLE_LIBRARY} won't be in ${LIBDEPS} on line 174? So why not leave the following two lines both where they were originally, i.e. on lines 136+137, and not duplicate them?

set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES})
generate_rootmap("${MODULE}" "${LIBDEPS}" "${CMAKE_CURRENT_SOURCE_DIR}/${MODULE}LinkDef.h")

When you compile and run locally, do you specifically run the load_library_PWGDQdielectron and load_library_PWGDQreducedTree unit tests? Those are the cause of the timeout in CI.

Apart from this CMake change I explained above, nothing immediately jumps out at me as a possible cause of the timeout...

TimoWilken avatar Jul 04 '23 15:07 TimoWilken

Hi @jjungIKF!

I was referring to the following lines you added after the new if(KFParticle_FOUND) block in PWGDQ/dielectron/CMakeLists.txt:

set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES})
generate_rootmap("${MODULE}" "${LIBDEPS}" "${CMAKE_CURRENT_SOURCE_DIR}/${MODULE}LinkDef.h")

In your previous PR, I think you didn't delete the same lines further above (lines 136+137), but now it seems you have.

I'm still a bit confused why you've done this -- doesn't the set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES}) after the set(LIBDEPS ${LIBDEPS} ${KFPARTICLE_LIBRARY}) wipe that change out again?

Also, in this case, presumably ${KFPARTICLE_LIBRARY} won't be in ${LIBDEPS} on line 174? So why not leave the following two lines both where they were originally, i.e. on lines 136+137, and not duplicate them?

set(LIBDEPS ${ALIPHYSICS_DEPENCIES} ${ALIROOT_DEPENDENCIES} ${ROOT_DEPENDENCIES})
generate_rootmap("${MODULE}" "${LIBDEPS}" "${CMAKE_CURRENT_SOURCE_DIR}/${MODULE}LinkDef.h")

When you compile and run locally, do you specifically run the load_library_PWGDQdielectron and load_library_PWGDQreducedTree unit tests? Those are the cause of the timeout in CI.

Apart from this CMake change I explained above, nothing immediately jumps out at me as a possible cause of the timeout...

Thank you for the fast reply @TimoWilken To be completely honest, I had to copy these lines from CMakefile in vertexingHF directory in order to make it compile on my local machine. I implemented this change as I want to use the root implementation of the KFParticle instead of the one in AliRoot. I am not too familiar with how CMakefiles actually work and hoped that the same code would fix my problem as it did on my local machine. Regarding the unit test. I have to check the next time I build AliPhysics again. I do not specifically request any unit test but only run the test which are done with the default build command.

jjungIKF avatar Jul 04 '23 16:07 jjungIKF

Hi @jjungIKF, I've marked this PR as draft to avoid long delays for other PRs. Let me know if those unit tests work locally for you!

TimoWilken avatar Jul 05 '23 16:07 TimoWilken

Hi @jjungIKF, I've marked this PR as draft to avoid long delays for other PRs. Let me know if those unit tests work locally for you!

Hi @TimoWilken, so I had another look at it and both unit test go through without any issues and the time passed for the test seems about normal as well.

jjungIKF avatar Jul 06 '23 17:07 jjungIKF

@TimoWilken Do you have any other ideas on what i could check?

jjungIKF avatar Jul 17 '23 10:07 jjungIKF

+test

pzhristov avatar Feb 07 '24 11:02 pzhristov