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

Validation fixes - 10_6_X #27224

Merged
merged 3 commits into from Aug 6, 2019
Merged

Conversation

perrotta
Copy link
Contributor

PR description:

Backport of the fixes merged in master with #27116 for the UL release

Starting from some issue reported by the static analyzer, I think there are two real bugs in these Validation files which are fixed here:

  • L732 of Validation/EventGenerator/plugins/TauValidation.cc, where with the original formulation the case with two negative and one positive pion is never hit
  • The fix in Validation/EventGenerator/src/HepMCValidationHelper.cc

A few other minor adjustments are also included here.

Furthermore, in order to recycle and cherry-pick the same commits as in the master version, the very first commit in this PR applies clang-format to the files affected, as it was done already in the master version.

Changes are expected in the DQM output because of the fixes (the same as in the mother version of this PR)

PR validation:

It compiles

if this PR is a backport please specify the original PR:

backport of #27116

@cmsbuild cmsbuild added this to the CMSSW_10_6_X milestone Jun 17, 2019
@perrotta
Copy link
Contributor Author

type bug-fix

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 17, 2019

A new Pull Request was created by @perrotta for CMSSW_10_6_X.

It involves the following packages:

Validation/EventGenerator
Validation/MuonGEMHits

@andrius-k, @kmaeshima, @schneiml, @efeyazgan, @cmsbuild, @jfernan2, @agrohsje, @fioriNTU, @alberto-sanchez, @qliphy can you please review it and eventually sign? Thanks.
@jshlee this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 17, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/980/console Started: 2019/06/17 11:34

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-03846e/980/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3208236
  • DQMHistoTests: Total failures: 25
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3207877
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@perrotta
Copy link
Contributor Author

backport of #27116

@jfernan2
Copy link
Contributor

jfernan2 commented Jun 17, 2019

+1
as long as generator team finds it useful

@fabiocos
Copy link
Contributor

@jfernan2 apparently the comment must be in a separate line, otherwise the bot does not correctly set the status (it does not matter in this case, I take your signature)

@efeyazgan @qliphy @alberto-sanchez I imagine this would be useful for GEN validation during UL activities

@qliphy
Copy link
Contributor

qliphy commented Jul 11, 2019

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_0_X is complete. 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)

@fabiocos
Copy link
Contributor

fabiocos commented Aug 6, 2019

+1

@cmsbuild cmsbuild merged commit 4771790 into cms-sw:CMSSW_10_6_X Aug 6, 2019
@perrotta perrotta deleted the validationFixes106X branch August 6, 2019 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants