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

Subjet TagInfo updates #15331

Conversation

ferencek
Copy link
Contributor

@ferencek ferencek commented Jul 30, 2016

This PR adds new features to the SV TagInfo producer that allow making subjet TagInfos using simple cone-based instead of ghost association to fat jets. The code now also makes it possible to perform such association using PATified fat jets and subjets. To fully exploit this last feature it was necessary to update the PATJetUpdater so that PAT jets are cloned (not constructed from a reference) in order to preserve the reference to the original object the PAT jet was derived from (this is necessary for re-establishing references between updated PAT fat jets and subjets). Finally, minor changes were added to jetTools.py to properly configure the updated SV TagInfo producer.

This is purely an analysis level update. There should be no impact on the standard reconstruction or MiniAOD production.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ferencek (Dinko Ferenček) for CMSSW_8_1_X.

It involves the following packages:

PhysicsTools/PatAlgos
RecoBTag/SecondaryVertex

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @JyothsnaKomaragiri this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@ferencek ferencek force-pushed the SubjetTagInfosNoClustering-PR_from-CMSSW_8_1_0_pre9 branch from 518c455 to 874a937 Compare July 31, 2016 12:31
edm::Ptr<reco::Jet> jetPtr = jets->ptrAt(idx);
Jet ajet( edm::RefToBase<Jet>(jetRef.castTo<JetRef>()) );
const Jet * origJet = dynamic_cast<const Jet *>( jetRef.get() );
Jet ajet( *origJet );
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding only: can you explain the reason for these changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this preserve the existing UserFloats in a PAT jet, when updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mverzett: If a pat::Jets is constructed from an object reference, then this newly created pat::Jet will be pointing to this object from which it was constructed. This is quite bad when using the updateJetCollection() function because every updated pat::Jet will lose memory of the original object it was derived from. This is particularly relevant for fat jets and subjets. If you decided to update either fat jets or subjets, you will no longer be able to figure out which subjet belongs to which fat jet.

@ahinzmann: Yes.

@cvuosalo
Copy link
Contributor

cvuosalo commented Aug 2, 2016

@ferencek: Could you please provide me a test config so I can verify the operation of the new features?
Thanks.

@ferencek ferencek force-pushed the SubjetTagInfosNoClustering-PR_from-CMSSW_8_1_0_pre9 branch from 874a937 to d3f9d0a Compare August 3, 2016 22:06
@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 3, 2016

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

@ferencek
Copy link
Contributor Author

ferencek commented Aug 3, 2016

@cvuosalo, I added an updateJetCollection(...) example in PhysicsTools/PatAlgos/test/patTuple_updateJets_fromMiniAOD_cfg.py that makes use of the newly added features.

@slava77
Copy link
Contributor

slava77 commented Aug 5, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

{
const pat::Jet * subJet = dynamic_cast<const pat::Jet *>( subjets->at(sj).jet().get() );
if( !subJet )
edm::LogError("WrongJetType") << "Wrong jet type for input subjets. Please check that the input subjets are of the pat::Jet type.";
Copy link
Contributor

Choose a reason for hiding this comment

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

After this line, there needs to be a break; or else if...; otherwise there could be a segmentation fault.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2016

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

@ferencek
Copy link
Contributor Author

ferencek commented Aug 8, 2016

@cvuosalo, thanks for the comment. I push a new commit that should improve handling of wrong input jet types.

@cvuosalo
Copy link
Contributor

cvuosalo commented Aug 8, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2016

@cvuosalo
Copy link
Contributor

cvuosalo commented Aug 8, 2016

+1

For #15331 df9bab9

Analysis-level enhancement of subjet TagInfos for the SV TagInfo producer. There should be no change in monitored quantities.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-08-08-1100 show no significant differences, as expected. A new unit test is included in the PR to test the new features, and the test in Jenkins is successful.

@davidlange6 davidlange6 merged commit 8b77dcd into cms-sw:CMSSW_8_1_X Aug 9, 2016
@ferencek ferencek deleted the SubjetTagInfosNoClustering-PR_from-CMSSW_8_1_0_pre9 branch August 9, 2016 20:42
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

7 participants