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

guard the nPho histograms from double counting, should fix HI issues #6574

Merged
merged 1 commit into from Nov 28, 2014

Conversation

harmonicoscillator
Copy link
Contributor

In HI reco DQM, photons are being double-counted because in HI RECO we do not pre-select at the RECO level for isolated photons. Since most HI photons are not isolated according to the default cuts they have type==0 and get double-counted.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @richard-cms (R. Alex Barbieri) for CMSSW_7_3_X.

guard the nPho histograms from double counting, should fix HI issues

It involves the following packages:

DQMOffline/EGamma

@cmsbuild, @danduggan, @nclopezo, @deguio can you please review it and eventually sign? Thanks.
@rociovilar 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 you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@deguio
Copy link
Contributor

deguio commented Nov 25, 2014

+1
please submit this PR to 74x as well.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @ktf, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@ktf
Copy link
Contributor

ktf commented Nov 26, 2014

This was forward ported to #6612. Please close this one if you do not need it in CMSSW_7_3_X.

@deguio
Copy link
Contributor

deguio commented Nov 26, 2014

fix. needed in 73 as well.

{
nPho[cut][type][0]++;
nPho[cut][type][part]++;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just remove all of the added lines and replace [0] by [type] in the two lines above?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not my business, but that changes the logic, does it not?

On Thu Nov 27 2014 at 6:55:32 PM David Lange notifications@github.com
wrote:

In DQMOffline/EGamma/plugins/PhotonAnalyzer.cc:

@@ -887,8 +887,11 @@ void PhotonAnalyzer::analyze( const edm::Event& e, const edm::EventSetup& esup )
//filling photon histograms
nPho[cut][0][0]++;
nPho[cut][0][part]++;

  • nPho[cut][type][0]++;
  • nPho[cut][type][part]++;
  • if(type != 0)
  • {
  • nPho[cut][type][0]++;
    
  • nPho[cut][type][part]++;
    
  • }

why not just remove all of the added lines and replace [0] by [type] in
the two lines above?


Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/6574/files#r21006980.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah - indeed it does

@davidlange6
Copy link
Contributor

+1

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

5 participants