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

Build FairShip with the separated libTPythia6.so library #597

Merged
merged 4 commits into from
Feb 26, 2025

Conversation

antonioiuliano2
Copy link
Contributor

Dear all,

This pull request is related to the update for ROOT 6.32 without TPythia6 support:
ShipSoft/shipdist#102.

Basically, this pull request will tell FairShip to load the TPythia6 from the shared library, now separated from ROOT.

It is used by

  • TPythia6Generator (compiled in shipgen), and run_simScript.py which calls it
  • makeCascade.py
  • makeMuonDIS.py

Best Regards,
Antonio

@antonioiuliano2 antonioiuliano2 requested a review from a team as a code owner February 7, 2025 10:08
Copy link
Contributor

@olantwin olantwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also use find_package to specify TPythia6 as a dependency and use it to define include paths etc.

@olantwin
Copy link
Contributor

olantwin commented Feb 7, 2025

Builds fine on SLC9

@olantwin
Copy link
Contributor

@antonioiuliano2 Any updates on this?

@olantwin
Copy link
Contributor

It would be good to add a changelog entry. While nothing user-facing should change, this is quite a major under-the-hood change.

@olantwin
Copy link
Contributor

One thing we could try for backwards compatibility is to check whether ROOT has TPythia6, and if not search for the TPythia6 standalone package using find_package.
Right now you kind of do that implicitly by using only an additional include path, which might be blank. Unfortunately the lack of TPythia6 might only be noticed at runtime as a result.

@olantwin
Copy link
Contributor

olantwin commented Feb 13, 2025

I think once we confirm that this build fine with:

  • ROOT 6.30 with builtin TPythia6
  • ROOT 6.32 with standalone TPythia6

And does not build with:

  • ROOT 6.32 without standalone TPythia6

We can probably merge this first version.

A changelog entry would still be appreciated ;)

@olantwin
Copy link
Contributor

@antonioiuliano2 Have you tried generating some muon DIS events with this branch?

@antonioiuliano2
Copy link
Contributor Author

@antonioiuliano2 Have you tried generating some muon DIS events with this branch?

Yes, I did a test with the muonShieldOptimization/makeMuonDIS.py script and the default muConcrete.root file. Nothing out of the order to report.

It would be good to add a changelog entry. While nothing user-facing should change, this is quite a major under-the-hood change.

Added an entry to the CHANGELOG.

@olantwin
Copy link
Contributor

Ok, could you please rebase?

@olantwin olantwin merged commit 189fedf into master Feb 26, 2025
2 checks passed
@olantwin olantwin deleted the include_tpythia6 branch February 26, 2025 10:25
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