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 DeepCSV and DeepCMVA, removing JetProbability #261

Merged
merged 5 commits into from
Sep 25, 2017

Conversation

OlivierBondu
Copy link
Member

@OlivierBondu OlivierBondu commented Sep 22, 2017

tracked at #257

NB:

  • I don't think any of us uses JetProbability, and we don't have SF for it, but it can easily be added back...
  • no SF added at this time
  • I expect the checks to fail, we should need to updated the ref trees... I'll do that on Monday

Copy link
Member

@pieterdavid pieterdavid left a comment

Choose a reason for hiding this comment

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

Thanks, I didn't know these were available in the late 80 releases :-). The tests fail exactly as expected indeed, with missing/new branches

@swertz
Copy link
Contributor

swertz commented Sep 22, 2017

FYI, CMVA is not really supported, and I think DeepCMVA isn't either.

Also, the tagger tags are pfDeepCSVJetTags:probX, where X is b, bb, c, and udsg, and the sum of the first two is what actually defines the b-tagger.

@pieterdavid
Copy link
Member

pieterdavid commented Sep 23, 2017

Thanks @swertz . It turns out that we also misread "is available in the release" as "is saved in the miniAOD", so we need to add a bit of configuration to update the jets with these b-tags. I got something working yesterday evening, I will further test and clean it up on Monday.

- part of redoJEC, as rerunning b-tagging requires redoing the JEC
- replace ":" to "_" in the branch names, in (Fat)JetsProducer
@OlivierBondu
Copy link
Member Author

I pushed @pieterdavid changes which look fine to me 😄

Maybe @swertz should sign off on this one ?

@pieterdavid
Copy link
Member

Few more comments on my commits that @OlivierBondu just merged in his branch: there are a number of warnings printed, from the configuration

**************************************************************
b tagging needs to be run on uncorrected jets. Hence, the JECs
will first be undone for 'updatedPatJetsAK4PFchsNewJEC' and then applied to
'updatedPatJetsTransientCorrectedAK4PFchsNewJEC'.
**************************************************************
skipping pfDeepCSVJetTagsAK4PFchsNewJEC as it has already been loaded in the process
skipping pfDeepCSVJetTagsAK4PFchsNewJEC as it has already been loaded in the process
skipping pfDeepCSVJetTagsAK4PFchsNewJEC as it has already been loaded in the process
skipping pfDeepCSVJetTagsAK4PFchsNewJEC as it has already been loaded in the process
**************************************************************

which seem harmless, see this hypernews thread.

The other ones are these:

%MSG-w L3Absolute not found:  PATJetUpdater:updatedPatJetsAK4PFchsNewJEC  25-Sep-2017 14:09:33 CEST Run: 1 Event: 270939
L2L3Residual and L3Absolute are not part of the jetCorrFactors
of module patJetCorrFactorsAK4PFchsNewJEC. Jets will remain uncorrected.
%MSG
%MSG-w L3Absolute not found:  PATJetUpdater:updatedPatJetsAK8PFchsNewJEC  25-Sep-2017 14:09:41 CEST Run: 1 Event: 270940
L2L3Residual and L3Absolute are not part of the jetCorrFactors
of module patJetCorrFactorsAK8PFchsNewJEC. Jets will remain uncorrected.
%MSG

which look a bit worrisome to me, but no changes in the momenta etc., so I hope this is a red herring as well (it's a bit hard to understand what it really means - but I did not dig deeply into the code that produces it yet).

@pieterdavid
Copy link
Member

One last point: @alesaggio @OlivierBondu don't forget to update the ZA config (passing the list of DeepCSV outputs to redoJEC, like here) as well, otherwise all values will be -1000 ;-)

@swertz
Copy link
Contributor

swertz commented Sep 25, 2017

Looks fine to me! About the strange logs: both messages are mentioned in the HN thread and are reported to be "normal"...

@pieterdavid pieterdavid merged commit b1458db into cp3-llbb:CMSSW_8_0_6p Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants