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.

Sign up for GitHub

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

Jump to bottom

Small update of FastPV sequence #5667

Merged
merged 5 commits into from Oct 7, 2014
Merged

Small update of FastPV sequence #5667

merged 5 commits into from Oct 7, 2014

Conversation

silviodonato
Copy link
Contributor

Small updates of the FastPrimaryVertex code and JetVertexChecker, presented at TSG meeting:
https://indico.cern.ch/event/338610/contribution/1/1/material/slides/0.pdf

In both Producer is possible to maintaing the "old" method, with the options:

  • "newMethod" in JetVertexChecker
  • "m_ptWeighting" in FastPrimaryVertexWithWeightsProducer

@cmsbuild cmsbuild added this to the Next CMSSW_7_3_X milestone Oct 3, 2014
@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2014

A new Pull Request was created by @silviodonato for CMSSW_7_3_X.

Small update of FastPV sequence

It involves the following packages:

RecoPixelVertexing/PixelVertexFinding

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @cerati, @GiacomoSguazzoni, @rovere, @VinInn 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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@silviodonato
Copy link
Contributor Author

added @arizzi

This was referenced Oct 3, 2014
Closed
Merged
StoyanStoynev
StoyanStoynev reviewed Oct 3, 2014
View reviewed changes
RecoPixelVertexing/PixelVertexFinding/src/FastPrimaryVertexWithWeightsProducer.cc
@@ -360,6 +369,7 @@ FastPrimaryVertexWithWeightsProducer::produce(edm::Event& iEvent, const edm::Eve
//calculate the final weight
weight= m_EC_weight*(weight_dPhi) ;
}
if(m_ptWeighting) weight=weight*(pt-m_ptWeighting_parameter)/(40-m_ptWeighting_parameter); // a jetPt=40 doesn't change weight
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this '40' and the one in '("minJetPt",40.)' related?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, m_ptWeighting_parameter is just a parameter used to parametrize the pT weight.

Using (pt-m_ptWeighting_parameter)/(40-m_ptWeighting_parameter) the pT weight is one for a jet with pT=40, just to put a momentum scale

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2014

Pull request #5667 was updated. @cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2014

+1
Tested at: a7ba604
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5667/151/summary.html

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2014

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2014

Pull request #5667 was updated. @cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2014

+1
Tested at: 44d4750
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5667/173/summary.html

@StoyanStoynev
Copy link
Contributor

+1
For 44d4750 .
I tested 390c4d3 and the difference is only the removal of a float (originally introduced by this PR) from the Event. So no changes to the event content. Essentially the PR introduces options which are off by default. No difference in reco in short matrix tests as well as other ttbar wf. Also based on #5670 (identical PR for 72X) - jenkins there are ready, fine, and show no difference (none expected as said).

@StoyanStoynev
Copy link
Contributor

My tests were on top of CMSSW_7_3_X_2014-09-30-0200.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 7, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

davidlange6 added a commit that referenced this pull request Oct 7, 2014
@davidlange6
Merge pull request #5667 from silviodonato/fastpv-ptweight
790c5f3 
Small update of FastPV sequence
@davidlange6 davidlange6 merged commit 790c5f3 into cms-sw:CMSSW_7_3_X Oct 7, 2014
@perrotta perrotta mentioned this pull request Oct 8, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-pending fully-signed reconstruction-approved tests-approved
Projects
None yet
Milestone
Next CMSSW_7_3_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@silviodonato @cmsbuild @StoyanStoynev @slava77 @davidlange6