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

[WIP] Fix on destructors not being called by o2sim #13769

Closed
wants to merge 8 commits into from

Conversation

jackal1-66
Copy link
Collaborator

Most generators destructors are default ones, however by using the HepMC generator I noticed that the destructor was not called when the simulation terminates. This called for a series of modifications:

  • Replaced new operator for generator instances with smart pointers (template of vectors of unique_ptr)
  • Modification of child process spawning in GeneratorFileorCmd => if this was not done then there was no way of killing the child process which could be left hanging writing on file while the full simulation is shutting down (this was a clear problem when using EPOS4 example).
  • Edited GeneratorHepMC destructor in order to call terminateCmd() BEFORE removing the temporary file (if cmd is not empty)

Fundamental to be able to kill the child process spawned by CMD functionality of GeneratorFileOrCmd generator. This was not so useful when the destructors were not called at the end of the simulation
@jackal1-66 jackal1-66 requested a review from sawenzel as a code owner December 4, 2024 15:37
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

@alibuild
Copy link
Collaborator

alibuild commented Dec 4, 2024

Error while checking build/O2/fullCI for 03913e4 at 2024-12-09 08:06:

## sw/BUILD/O2Physics-latest/log
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/13769-slc8_x86-64/0/Generators/src/GeneratorFileOrCmd.cxx:20:10: error: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead [modernize-deprecated-headers]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@jackal1-66
Copy link
Collaborator Author

GeneratorService is failing, the PR requires more adjustments, not to be merged for the moment

@alibuild
Copy link
Collaborator

alibuild commented Dec 9, 2024

Error while checking build/O2/fullCI_slc9 for 22bfb67 at 2024-12-09 12:42:

## sw/BUILD/O2-latest/log
ninja: build stopped: subcommand failed.

Full log here.

@alibuild
Copy link
Collaborator

alibuild commented Dec 10, 2024

Error while checking build/O2/fullCI for 22bfb67 at 2024-12-11 17:38:

## sw/BUILD/O2-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/13769-slc8_x86-64/0/Generators/src/GeneratorFileOrCmd.cxx:20:10: error: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead [modernize-deprecated-headers]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@jackal1-66 jackal1-66 changed the title Fix on destructors not being called by o2sim [WIP] Fix on destructors not being called by o2sim Dec 11, 2024
@jackal1-66
Copy link
Collaborator Author

Setting [WIP] in the PR title. O2sim tests work, but GeneratorService test fails and more debug is needed.

Cleanup function called manually in main simulation programs
@jackal1-66 jackal1-66 requested a review from a team as a code owner December 12, 2024 15:50
@sawenzel
Copy link
Collaborator

Closing. Alternative PR #13817 merged.

@sawenzel sawenzel closed this Dec 19, 2024
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