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

Adding no-HF grid-based rho (76x forward-port #11773), and fixing existing bugged central rho values #11855

Merged
merged 8 commits into from
Oct 23, 2015

Conversation

rappoccio
Copy link
Contributor

In the forward-port of #11773 now adding the fixed grid central version explicitly.

Also, found a bug in the central "charged pileup" and "neutral hadron" rho values. They incorrectly overrode the "particleFlow" input source, so ALL of these were exactly the same. Now fixed.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rappoccio for CMSSW_7_6_X.

Adding no-HF grid-based rho : 76x forward-port #11773

It involves the following packages:

RecoJets/Configuration

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @ahinzmann, @jdolen, @nhanvtran, @schoef, @mariadalfonso this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented Oct 17, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/8963/console

@cmsbuild
Copy link
Contributor

-1

Tested at: 0a57593
I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11855/8963/summary.html

@cvuosalo
Copy link
Contributor

@cmsbuild please test
Hoping the previous Jenkins failure is transient

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/8968/console

@cmsbuild
Copy link
Contributor

-1

Tested at: 0a57593
I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11855/8968/summary.html

@cvuosalo
Copy link
Contributor

+1

For #11855 0a57593

Adding an eta-restricted (no HF) grid-based rho for future use in Mini-AOD or RECO. There should be no other change in monitored quantities. #11771 and #11773 are the 74X and 75X versions of this PR, and they have already been approved by Reco.

The code changes are satisfactory. Jenkins tests failed to due errors in the IB unrelated to this PR. Local matrix tests against baseline CMSSW_7_6_0_pre6 show no significant differences, as expected. The addition of the new quantity is confirmed by the tests, but the product size increase is negligible.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (but tests are reportedly failing). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@rappoccio
Copy link
Contributor Author

I don't see the errors. Is this an xrootd problem or something?

@davidlt
Copy link
Contributor

davidlt commented Oct 19, 2015

The process was terminated before it managed to print anything else, .e.g it was killed due to timeout.

@rappoccio
Copy link
Contributor Author

Thanks David, I'll wait for a retest then.

@smuzaffar
Copy link
Contributor

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9034/console

@rappoccio rappoccio changed the title Adding no-HF grid-based rho : 76x forward-port #11773 Adding no-HF grid-based rho (76x forward-port #11773), and fixing existing bugged central rho values Oct 19, 2015
@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Oct 21, 2015

NB: based on git grep, fixedGridRhoFastjetCentralChargedPileUp and fixedGridRhoFastjetCentralNeutral are not used in the code available in CMSSW.

@rappoccio
Copy link
Contributor Author

Correct, this is not used except for analysis until now.

@cvuosalo
Copy link
Contributor

+1

For #11855 22074ba

Second approval after rebasing and minor addition. The change is satisfactory, and Jenkins tests against baseline CMSSW_7_6_X_2015-10-19-1100 show no significant differences, as expected.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Oct 23, 2015
Adding no-HF grid-based rho (76x forward-port #11773), and fixing existing bugged central rho values
@cmsbuild cmsbuild merged commit 167604e into cms-sw:CMSSW_7_6_X Oct 23, 2015
@rappoccio rappoccio deleted the rappoccio_76x_gridrhoNoHF branch February 3, 2018 17:03
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.

7 participants