-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
revert stream::producer to simple producer to avoid crashes in HTXSRivetProducer #22025
Conversation
The code-checks are being triggered in jenkins. |
please test |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22025/3123 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22025/3123/git-diff.patch You can run |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22025/3155 |
A new Pull Request was created by @perrozzi for master. It involves the following packages: GeneratorInterface/RivetInterface @cmsbuild, @efeyazgan, @perrozzi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@Dr15Jones this is the PR I was mentioning at the end of the ORP meeting. |
Do you know for certain that Rivet does not use any global variables? If it does, then this can't be a stream module. You should probably make it a |
The code-checks are being triggered in jenkins. |
The code-checks are being triggered in jenkins. |
The tests are being triggered in jenkins. |
Pull request #22025 was updated. @cmsbuild, @efeyazgan, @perrozzi can you please check and sign again. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
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) |
@perrozzi how was this validated? Resuming a past comment of mine about Rivet validation, could we setup a unit test that runs at least one analysis and compares the output to a given reference? At present we are essentially blind and relying on private checks... |
@fabiocos can the PR be merged asap, as it relevant for the nanoAOD production? |
@perrozzi I ran on a GluGluHToGG_M125_13TeV_amcatnloFXFX_pythia8_94X_mc2017_realistic_v10-v1 file in CMSSW_9_4_4, using GeneratorInterface/RivetInterface/test/runRivetHTXS_cfg.py as provided in the release (except for input file). The code compiled, the start and end messages were as expected, and the contents of the output file were as expected. |
+1 |
It has been observed that the use of
stream::EDProducer
causes a crash in the Higgs Template Cross Section Rivet producer. The change had not been captured by the validation as no dedicated relval workflow is setup.Reverted to simple
EDProducer
@dsperka @sethzenz