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 #11771

Closed

Conversation

rappoccio
Copy link
Contributor

As per request of PPD and the collaboration, this is the addition of an eta-restricted (no HF) grid-based rho. This can be used for miniAOD and/or future reco.

@cmsbuild
Copy link
Contributor

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

Adding no-HF grid-based rho

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 13, 2015

@cmsbuild please test

@slava77
Copy link
Contributor

slava77 commented Oct 13, 2015

@rappoccio
please clarify the scope of this for 74X, especially given the mentioned request of the Collaboration.
Is this for some targeted reprocessing or for something else?

@cmsbuild
Copy link
Contributor

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

@rappoccio
Copy link
Contributor Author

@slava77 I'm not sure where this will go. @gpetruc @arizzi @bachtis have much more information.

The goal is to have an "HF-less silver lumi list" to recover the data with HF problems. This would be needed to analyze those data (and derive JECs).

@slava77
Copy link
Contributor

slava77 commented Oct 13, 2015

at least this morning there was no discussion of another re-miniAOD.
It's unclear to me how this modification will work.
(I can see a recipe that starts from miniAOD, but that's not what's in this PR)

@rappoccio
Copy link
Contributor Author

The details were not clear to me either. If we end up with a miniAOD "silver version" then that is a better place, I agree.

@@ -61,6 +61,9 @@
)


fixedGridRhoFastjetCentral = fixedGridRhoFastjetAll.clone(
maxRapidity = cms.double(2.5)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose if that recipe includes
from RecoJets/Configuration/RecoPFJets_cff import fixedGridRhoFastjetCentral
it would still kind of work.
Still, considering the inclusive keep *_fixedGridRho*_*_* that we have in in the RecoJets_EventContent_cff, we will have to at least make a new Prompt version or wait until all PromptReco with pp is done to include this in the mainstream release.
The former is probably more feasible, but still not that desired.

Please put the definition of fixedGridRhoFastjetCentral in a separate file (in 74X only)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if we're going to go that route we should just put this into any "silver" miniAOD file directly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe; it would also probably have a different src as well (packedCandidates)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so maybe it's best to close this and do it via miniAOD or something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can stay open, just don't expect it to go far ;)
There are some requests building up for JetMET limited reprocessing in 74X, which may use this PR as well.
It's not clear to me still when and how that will be made (a branched-off special 74_1X release, probably)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I will leave this "as is" and we can decide what to do later.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #11771 eebcf82

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.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_7_4_X_2015-10-13-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_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@slava77
Copy link
Contributor

slava77 commented Oct 18, 2015

@davidlange6
if this gets merged for the prompt reco, we should change the dataset versions

@cmsbuild
Copy link
Contributor

Pull request #11771 was updated. @cmsbuild, @cvuosalo, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Oct 24, 2015

davidlange6 modified the milestone: Next CMSSW_7_4_X, Next CMSSW_7_6_X 9 hours ago

David,
Does it make sense?
Why does the PR submitted to cms-sw:CMSSW_7_4_X now show up with a 76X as a milestone?

@davidlange6
Copy link
Contributor

@smuzaffar - something definitely went wrong here. Not sure what button I could have pushed to accomplish this?

@cmsbuild cmsbuild modified the milestones: Next CMSSW_7_4_X, Next CMSSW_7_6_X Oct 24, 2015
@slava77
Copy link
Contributor

slava77 commented Nov 13, 2015

-1
to keep this off the list of items pending RECO review
there are no plans for major reprocessing in 74X; the PR affects the standard sequences.
It's still a good reference for recipes in analyses.

@rappoccio rappoccio deleted the rappoccio_74x_gridrhoNoHF branch February 3, 2018 17:02
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.

5 participants