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

Pythia8 seeding improvements #1695

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

Conversation

sawenzel
Copy link
Contributor

simplification of seeding code and harmonization thereof

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

@sawenzel sawenzel force-pushed the swenzel/pythia8_seeding_impr branch from 976b89b to 5eeeec4 Compare July 10, 2024 14:35
Comment on lines 229 to +231
auto seed = (gRandom->TRandom::GetSeed() % 900000000);
myGen->setUsedSeed(seed);
myGen->readString("Random:setSeed on");
myGen->readString("Random:seed " + std::to_string(seed));
myGen->setInitialSeed(seed);
Copy link
Contributor

@fcatalan92 fcatalan92 Jul 10, 2024

Choose a reason for hiding this comment

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

Hi @sawenzel, if I understood correctly the implementation in AliceO2Group/AliceO2#13281, this part is now redundant and can be removed, since it does exactly the steps performed when no specific seed is provided (i.e. takes it from ROOT)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @fcatalan92 : Yes, I think we could simplify this further by taking away even the auto seed = ; ... setInitialSeed() steps. But I wasn't sure if this breaks something because of setUsedSeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, after some recent developments for Pb-Pb HF productions, setUsedSeed is used to store the seed in our custom generator. This seed is later accessed by generator_pythia8_embed_hf.C to select a pT-hard bin.

auto seed = dynamic_cast<GeneratorPythia8GapTriggeredHF*>(mGeneratorEvHF)->getUsedSeed();

I wonder if a getter to the used seed could be added to GeneratorPythia8, this will allow us to simplify our custom generators. In this case, I think the seed should be updated when calling GeneratorPythia8::seedGenerator().

At this point, all looks good to me for this PR. I can take care of updating the HF generators later on, if the development I proposed is done.

@sawenzel sawenzel force-pushed the swenzel/pythia8_seeding_impr branch from 5eeeec4 to f82b452 Compare July 11, 2024 06:12
This commit harmonizes the seeding procedure of generators based
on o2::eventgen::GeneratorPythia8, following a development done in O2
(AliceO2Group/AliceO2#13281)

In principle, o2::eventgen::GeneratorPythia8 now performs it's own
seeding within the Init function. This seeding makes sure
to integrate with the `--seed` command line option of our event generation
executable.

The commit:
* Removes seeding code in O2DPG when it would override the default
  behaviour from O2.

* Uses a new dedicated function `setInitialSeed` instead of manipulating
  Pythia8 with config strings (in situations in which the user
  targets seeding based on ALIEN_PROC_ID for instance).

* Ensures that event generation becomes more repeatable in situations
  when the same `--seed` is given to the event generation executable.
  This may help to debug crashes on the GRID etc.
@sawenzel sawenzel force-pushed the swenzel/pythia8_seeding_impr branch from f82b452 to 81f99b3 Compare July 11, 2024 07:10
Comment on lines 229 to +231
auto seed = (gRandom->TRandom::GetSeed() % 900000000);
myGen->setUsedSeed(seed);
myGen->readString("Random:setSeed on");
myGen->readString("Random:seed " + std::to_string(seed));
myGen->setInitialSeed(seed);
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, after some recent developments for Pb-Pb HF productions, setUsedSeed is used to store the seed in our custom generator. This seed is later accessed by generator_pythia8_embed_hf.C to select a pT-hard bin.

auto seed = dynamic_cast<GeneratorPythia8GapTriggeredHF*>(mGeneratorEvHF)->getUsedSeed();

I wonder if a getter to the used seed could be added to GeneratorPythia8, this will allow us to simplify our custom generators. In this case, I think the seed should be updated when calling GeneratorPythia8::seedGenerator().

At this point, all looks good to me for this PR. I can take care of updating the HF generators later on, if the development I proposed is done.

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