Properly handle Run product merging in LHESource #21820
Conversation
Previously, when the first xml document was finished and the XMLDocument object was deleted, then xerces would be uninitialized. This lead to the parser being invalid so the next read of LHEReader would crash. Keeping the XercesPlatform alive for the lifetime of the LHEReader fixes that problem.
Moved to std::unique_ptr and std::shared_ptr for most memory handling. Changed member variables to match CMS conventions.
When we identify that we need to merge the LHERunInfoProduct in nextEvent, we immediately do the merge. This avoids the need for a endRun call which will allow those calls to be removed from the framework.
The function fileIndex was not being overridden for base class ProducerSourceBase. Now we forward FromFile::fileIndex to ProducerSourceBase.
By telling the framework that the LHESource has gone to a new file, the merging of RunInfoProduct can be done properly.
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21820/2817 |
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: FWCore/Sources @cmsbuild, @smuzaffar, @efeyazgan, @Dr15Jones, @perrozzi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
| @@ -0,0 +1,153 @@ | |||
| Number of particles = 12 | |||
There was a problem hiding this comment.
hi, is this log file needed?
There was a problem hiding this comment.
It is used by the unit test. The test compares the output of the test job to this log file and if there is a difference, the test fails.
| @@ -0,0 +1,197 @@ | |||
| Number of particles = 12 | |||
There was a problem hiding this comment.
This is also used by the unit test.
|
+1 |
cmsbuild
added
fully-signed
generators-approved
and removed
generators-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
Fixes issue #21807 |
|
please test workflow 513.0 |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1 |
The LHESource was not properly handling the merging of the LHERunInfoProduct when multiple files are read. The framework requires that the source explicilty tell the framework a new file is being opened which setup the system for proper merging.
This change also fixed a crash in the LHESource which happened if multiple files were read.