Skip to content

Conversation

@pbuehler
Copy link
Collaborator

@pbuehler pbuehler commented Dec 4, 2024

Without this the particle buffer mParticles is continuously growing.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2024

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

@jackal1-66
Copy link
Collaborator

jackal1-66 commented Dec 4, 2024

Hello @pbuehler, this is done already in Generator::ReadEvent. Could you please provide an example in which you're experiencing such a behaviour? When running o2-sim and the o2dpg machinery I don't see it with EPOS4, JETSCAPE or STARlight.

Copy link
Collaborator

@jackal1-66 jackal1-66 left a comment

Choose a reason for hiding this comment

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

Done already in ReadEvent, where do you get a not cleared particles array?

@sawenzel
Copy link
Collaborator

sawenzel commented Dec 5, 2024

For me this looks ok. Clearing the vector cannot hurt, or can it? However, it would be good having a reproducer of the bug, because I also agree with @jackal1-66 that the vector seems to be cleared in a higher level framework function. If something goes wrong there, we might have a more serious logic problem. @pbuehler : Do you have a reproducer?

sawenzel
sawenzel previously approved these changes Dec 5, 2024
@pbuehler
Copy link
Collaborator Author

pbuehler commented Dec 5, 2024

For me this looks ok. Clearing the vector cannot hurt, or can it? However, it would be good having a reproducer of the bug, because I also agree with @jackal1-66 that the vector seems to be cleared in a higher level framework function. If something goes wrong there, we might have a more serious logic problem. @pbuehler : Do you have a reproducer?

@pbuehler pbuehler closed this Dec 5, 2024
@pbuehler
Copy link
Collaborator Author

pbuehler commented Dec 5, 2024

I'll try to provide a simple example.

@pbuehler
Copy link
Collaborator Author

pbuehler commented Dec 5, 2024

Hi @sawenzel , @jackal1-66,
below I send you an example code which reproduces the issue. The first few lines provide instructions. Make sure that you replace the HepMC file name (fname) with an existing one.
It seems that with repeated execution of
gen->generateEvent()
gen->importParticles();

the number of particles in the output continuous to increase.

// to test do ...
// create a file HepMCReader.C with the following code:
// then in root do ..
// .L HepMCReader.C
//
// auto gen = new o2::eventgen::HepMCReader_class();
// std::string fname("ALICE_Con_pipi.hepmc3");
// gen->openHepMCFile(fname);
//
// 1. event
// gen->generateEvent()
// gen->importParticles();
// 2. event
// gen->generateEvent()
// gen->importParticles();

namespace o2 {
namespace eventgen {
class HepMCReader_class : public Generator {
public:
HepMCReader_class() { };
~HepMCReader_class() = default;

bool openHepMCFile(std::string fname) {
bool stat = true;

//delete reader;
reader = new o2::eventgen::GeneratorHepMC();
reader->setFileNames(fname);
std::cout << "GenerateEvent: Graniitti class/object is ";
if (!reader->Init()) {
  stat = false;
  std::cout << "not ";
}
std::cout << "properly initialized."
          << std::endl;
return stat;

};

bool generateEvent() override {
return reader->generateEvent();
};

bool importParticles() override {
if (!reader->importParticles()) {
return false;
}
printParticles();

return true;

};

void printParticles()
{
std::cout << "\n\n";
std::cout << "This event has " << reader->getParticles().size() << " particles\n\n";
for (auto& particle : reader->getParticles()) particle.Print();
}

private:
o2::eventgen::GeneratorHepMC *reader = 0x0;

};

} // namespace eventgen
} // namespace o2

@pbuehler pbuehler reopened this Dec 5, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2024

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

Return with false instead of stopping execution.
@sawenzel
Copy link
Collaborator

sawenzel commented Dec 5, 2024

Ok I see. This is because not everyone is calling ReadEvent but one may choose to call generateEvent + importParticles.
Should probably take a systematic look into all generators.

@jackal1-66
Copy link
Collaborator

@sawenzel most of the generators are not calling the function in importParticles. The only ones which do this for the moment are BoxGenerator and Hybrid (from a fast search in the repo). Indeed if ReadEvent is not called the array won't be cleared and this PR becomes a necessity (as it is for the other generators).

@sawenzel sawenzel merged commit 4bd54a7 into AliceO2Group:dev Dec 6, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants