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

L1 pf jets matching - Adding the template module #19068

Merged
merged 9 commits into from Jun 8, 2017

Conversation

vukasinmilosevic
Copy link
Contributor

Follow up to the #18992. Made a template and integrated latest comments.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

A new Pull Request was created by @vukasinmilosevic (Vukasin Milosevic) for master.

It involves the following packages:

RecoTauTag/HLTProducers

@Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@vukasinmilosevic vukasinmilosevic changed the title Adding the template module L1 pf jets matching - Adding the template module Jun 1, 2017
for (unsigned int i = 0; i < pfMatchedJets.size()-1; i++)
for (unsigned int j = i+1; j < pfMatchedJets.size(); j++)
{
const T & myJet1 = (pfMatchedJets)[i];
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this directly after the first for loop with index i

for(unsigned int iJet = 0; iJet < pfJets->size(); iJet++){
for(unsigned int iL1Jet = 0; iL1Jet < jetCandRefVec.size(); iL1Jet++){
// Find the relative L2pfJets, to see if it has been reconstructed
const T & myJet = (*pfJets)[iJet];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: move to outer for loop

desc.add<double> ("pt2_Min",35.0)->setComment("Minimal pT2 of PFJets to match");
desc.add<double> ("mjj_Min",650.0)->setComment("Minimal mjj of matched PFjets");
descriptions.setComment("This module produces collection of PFJetss matched to L1 Taus / Jets passing a HLT filter (Only p4 and vertex of returned PFJetss are set).");
descriptions.add ("L1TJetsMatching",desc);
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be done differently for templated classes as otherwise all instances generate a cfi with the same name.
something like:

#include "HLTrigger/HLTcore/interface/defaultModuleLabel.h"
descriptions.add(defaultModuleLabel<L1TJetsMatching<T>>(), desc);


typedef L1TJetsMatching<reco::PFJet> L1PFTJetsMatching ;
typedef L1TJetsMatching<reco::CaloJet> L1CaloTJetsMatching ;

Copy link
Contributor

Choose a reason for hiding this comment

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

L1T.... instead of L1.... (new L1T convention)

@@ -45,4 +52,6 @@ DEFINE_FWK_MODULE(VertexFromTrackProducer);
//DEFINE_FWK_MODULE(L2TauPixelTrackMatch);
DEFINE_FWK_MODULE(HLTPFTauPairLeadTrackDzMatchFilter);
DEFINE_FWK_MODULE(L2TauPixelIsoTagProducer);
DEFINE_FWK_MODULE(L1CaloTJetsMatching);
DEFINE_FWK_MODULE(L1PFTJetsMatching);

Copy link
Contributor

Choose a reason for hiding this comment

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

L1T... instead of L1...

@Martin-Grunewald
Copy link
Contributor

Please close the other two PRs!

// Find the relative L2pfJets, to see if it has been reconstructed
const T & myJet = (*pfJets)[iJet];
// if ((iJet<3) && (iL1Jet==0)) std::cout<<myJet.p4().Pt()<<" ";
deltaR = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (jetCandRefVec[iL1Jet]->p4()).Vect());
Copy link
Contributor

Choose a reason for hiding this comment

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

Quoting David Lange

please use deltaR2 from DataFormats/Math/interface/deltaR unless some other functionality is intended

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

Pull request #19068 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20295/console Started: 2017/06/02 06:44

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2017

Comparison job queued.

for(unsigned int iL1Jet = 0; iL1Jet < jetCandRefVec.size(); iL1Jet++){
// Find the relative L2pfJets, to see if it has been reconstructed
// if ((iJet<3) && (iL1Jet==0)) std::cout<<myJet.p4().Pt()<<" ";
deltaR = reco::deltaR2(myJet.p4().Vect(), (jetCandRefVec[iL1Jet]->p4()).Vect());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think deltaR2 returns deltaR^2... please check and then square the cut value (once at the beginning) and compare, rather than taking the sqrt, to be faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, deltaR2 is deltaR^2

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2017

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19068/20295/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 1680 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1789200
  • DQMHistoTests: Total failures: 52142
  • DQMHistoTests: Total nulls: 95
  • DQMHistoTests: Total successes: 1736790
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@davidlange6
Copy link
Contributor

davidlange6 commented Jun 8, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2017

Pull request #19068 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20425/console Started: 2017/06/08 14:25

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2017

Comparison job queued.

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2017

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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2017

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19068/20425/summary.html

Comparison Summary:

  • You potentially added 953 lines to the logs
  • Reco comparison results: 4604 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1840327
  • DQMHistoTests: Total failures: 22669
  • DQMHistoTests: Total nulls: 193
  • DQMHistoTests: Total successes: 1817292
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 0d39671 into cms-sw:master Jun 8, 2017
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

6 participants