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

PUPPI jets in RECO #28936

Merged
merged 57 commits into from Mar 24, 2020
Merged

PUPPI jets in RECO #28936

merged 57 commits into from Mar 24, 2020

Conversation

ahinzmann
Copy link
Contributor

@ahinzmann ahinzmann commented Feb 12, 2020

PR description:

Followup from #28659 and #28077 @rappoccio

This PR computes puppi weights and puppi AK4 and AK8 jets in RECO and stores the PUPPI weight as a value map. PUPPI jets are clustered based on standard PF candidates together with the value map of weights. A new boolean "isWeighted" is added to the reco::Jet interface to indicate whether weights were applied during jet clustering. The packed candidates pick up the value map such that puppi does not need to be re-computed in MiniAOD.

The same can be done for the puppiNoLep and PUPPI MET in a separate PR (to keep this one as slim as possible).

Other changes:

  • Removing AK8CHS from RECO
  • Removing CMSTopTagger from RECO
  • slimmedJetsPuppi: Removing puppi multiplictiy user floats and store the directly in jet specifics rounded to integers (this is enough for jet ID purposes anyways).
  • slimmedJetsAK8: Remove all user float related to AK8CHS
  • Implement BoostedDoubleSVProducer::fillDescriptions
  • Fix HLT configuration of hltBoostedDBSVAK8TagInfosPF in HLT_GRun_cff. This needs to be followed up and fixed everywhere in HLT Misconfiguration of hltBoostedDBSVAK8TagInfosPF #29203.
  • HOEnergyFraction for PUPPI jets is adapted to account for the PUPPI weight

PR validation:

runTheMatrix -l limited

if this PR is a backport please specify the original PR and why you need to backport that PR:

No backport planned.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@silviodonato
Copy link
Contributor

Kind reminder
dqm: @andrius-k @jfernan2 @kmaeshima @fioriNTU @schneiml
analysis: @santocch

@jfernan2
Copy link
Contributor

jfernan2 commented Mar 23, 2020

@silviodonato I have commented several times about this PR since the beginning, asking for an understanding of the differences. If people are fine with the modifications I am fine too, but I don't understand them. I quote here again my last comment:
Changes in DQM related to the transition to PUPPI.

  • Some plots in B2G and Exotica folders modified and filled empty. FYI: @avartak @rappoccio @clelange @pmaksim1 please consider to remove or fix them
  • @ahinzmann There are some modifications now in HLT folder for wf 11634.0 and 12434.0 for B2G, BTV and JME which I don't recall in previous tests of this PR but I cannot see them since those tests have been removed due to age, I guess that they are also related to the transition to PUPPI

@ahinzmann
Copy link
Contributor Author

* @ahinzmann There are some modifications now in HLT folder for wf 11634.0 and 12434.0 for B2G, BTV and JME which I don't recall in previous tests of this PR but I cannot see them since those tests have been removed due to age, I guess that they are also related to the transition to PUPPI

The reference offline jet collection for all HLT plots using AK8 was changed from CHS to PUPPI. This should explain the observed differences in the HLT plots.

@jfernan2
Copy link
Contributor

+1

@silviodonato
Copy link
Contributor

merge
in order to be included in 11_1_0_pre5
@santocch please let me know if you have any objections

@silviodonato
Copy link
Contributor

we are getting a new error in CMSSW_11_1_X_2020-03-24-2300 wf 25202.01
https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/raw/slc7_amd64_gcc820/CMSSW_11_1_X_2020-03-24-2300/pyRelValMatrixLogs/run/25202.01_TTbar_13+TTbar_13INPUT+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15MC_PU25_JME/step5_TTbar_13+TTbar_13INPUT+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15MC_PU25_JME.log

----- Begin Fatal Exception 25-Mar-2020 06:14:22 CET-----------------------
An exception of category 'UnknownUserFloat' occurred while
   [0] Processing  Event run: 1 lumi: 37 event: 1802 stream: 1
   [1] Running path 'nanoAOD_step'
   [2] Calling method for module SimpleCandidateFlatTableProducer/'AK4PFPUPPITable'
Exception Message:
Requested UserFloat patPuppiJetSpecificProducer:neutralPuppiMultiplicity is not available! Possible UserFloats are: 
chFPV0EF chFPV1EF chFPV2EF chFPV3EF 
----- End Fatal Exception -------------------------------------------------

@ahinzmann
Copy link
Contributor Author

ahinzmann commented Mar 25, 2020

The fix for this error is now in here in #29254:
1d010b8
These variables are removed from MiniAOD by #28936 and should not enter JME NanoAOD.

@perrotta
Copy link
Contributor

@peruzzim , please have a look

Let me point out that the error does not necessarily comes from this PR. In the same CMSSW_11_1_X_2020-03-24-2300 IB thr bug fix PR #28854 was also merged, which also fixes a bug which prevented 25202.01.
This specific error was not observed in the automatic tests of #28854 that run 25202.01: however, the input events in that test were generated on the fly, while in the IB they are read from store, and they fail at event 4/10.

@peruzzim
Copy link
Contributor

I struggle to follow all these changes done incoherently to the custom JME configuration.

What I know happened: in #29212 I changed some JME producer, for which a minor fix contained in #29257 was needed. I think this is a closed incident, workflow 25202.01 ran ok in the PR tests.

Then, checking the logs in the latest IB, I see that there is now a different issue that breaks the same workflow. @ahinzmann says this error was introduced by this PR (#28936), let's see if the fix proposed in #29296 will fix it. I don't know whether there is a problem in the design of this PR that does not allow anymore to run on older datasets without crashing. I hope not; if there is, it should be fixed.

I encourage @ahinzmann and others in JME to coordinate better when touching these things, because changes to this file in master have become so frequent that it's impossible for me to follow what happens there. I don't know if you want to consider some pre-integration of changes, on the model of what we do for NanoAOD production code. At least, please run this workflow systematically when you test a PR touching jets.

@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

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 be automatically merged.

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