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

Compile starlight against dpmjet #5680

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mbroz84
Copy link

@mbroz84 mbroz84 commented Nov 5, 2024

STARlight can be compiled against DPMJET, in that case a separate library is created (libDpmJetLib) that uses routines from DPMJET to simulate photo-nuclear interactions. UPC group would like to use this mode.

The libDpmJetLib is somehow not installed and remains in the build directory so I added explicit line to move it, maybe there is another solution.

There is an adjustment in STARlight itself needed to make this work. I can make the PR myself, but I will appreciate an expert opinion on issue 2.

  1. The libDpmJetLib was build as static, I added option to build it as Shared.
  2. There is an issue with "FIND_LIBRARY" in the CMake module when the library path defined as a environment variable in the alidist/starlight.sh is preceded by ENV. Library is then not found when this is build via aliBuild. It definitely works in standalone installation (to see what I mean exactly have a look in the attached patch).
    STARlight.patch

@jackal1-66
Copy link
Contributor

There might be an issue in building this (I tested it locally). I will start the CI to see if it appears also there.

@mbroz84
Copy link
Author

mbroz84 commented Nov 14, 2024

It needs this: alisw/DPMJET#7
And the attached STARlight patch

@jackal1-66
Copy link
Contributor

I included the DPMJET PR in my build, but I overlooked the STARlight patch you suggested. Indeed it doesn't build without that. Going to check now the suggested changes

@jackal1-66
Copy link
Contributor

I made some more tests. Your patch does not work with our current STARlight version, but that's not a big problem because we can update it to the last one available on STARlightsim. The modification for DPMJET as a shared library can be suggested directly as a PR in the main repository (it's probably a good idea to have it there). For the fact that STARlight is not able to find the library instead I'm trying to find alternative solutions, maybe a modification in the recipe will be required.

@mbroz84
Copy link
Author

mbroz84 commented Nov 14, 2024

I can send the PR to STARlightsim anytime (developers are our colleagues from ALICE). I have there the library build option for the time being, let me know if anything else is needed.

@jackal1-66
Copy link
Contributor

I discussed with @ktf and there's an easy way to do the second point: export DPMJET_DIR=$DPMJET_ROOT right before the cmake is done in the starlight recipe, this way you don't need to edit FindDPMJET.cmake.
To summarize:

  1. Open the PR with STARlight to for the dynamic library type of DPMJET: this way when it's merged we get directly the last version with the shared library enabled. This is, I would say, the first step so that we edit our own starlight recipe to reflect the generator, and we wait for the DPMJET PR to be merged and for the new tag
  2. Add the DPMJET_DIR export in the starlight recipe
  3. Fix the indentation in the recipe (please)
  4. Remove the dpmdata folder copy in the dpmjet recipe (it is done automatically already)
  5. When point 1 is fully completed we update the versions of both starlight and dpmjet in the recipes, we retest that the build works smoothly and we merge.

@mbroz84
Copy link
Author

mbroz84 commented Nov 17, 2024

  1. Done
  2. Done
  3. Done
  4. I also adjusted the tags in both recipies to point to the new versions.

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.

2 participants