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

Fix a chi2 bug for Method2 in 74X #7015

Merged

Conversation

lihux25
Copy link
Contributor

@lihux25 lihux25 commented Dec 28, 2014

There were bugs fixed in 73X (PR#6855) and were ported to 74X (PR#6891). However there was a remaining part fixed in 73X but not in 74X. This PR is for this fix.

…d to 74X (PR#6891). However there was a remaining part fixed in 73X but not in 74X. This PR is for this fix.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lihux25 (Hongxuan Liu) for CMSSW_7_4_X.

Fix a chi2 bug for Method2 in 74X

It involves the following packages:

RecoLocalCalo/HcalRecAlgos

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
@argiro 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.

@slava77
Copy link
Contributor

slava77 commented Jan 6, 2015

+1

for #7015 3a8cd2e
tested in CMSSW_7_4_X_2014-12-30-1400 (test area sign482)

The pedestal constraint part was indeed missed. Thanks for the fix.

Changes are generally small and become more visible in samples with pileup.
Here are some plots from 25202 (ttbar PU35@25ns)

all_sign482vsorig_ttbarpuwf25202p0c_log10hbherechitssorted_hbhereco__reco_obj_obj_energy
this propagates to caloTowers and other quantities as migration of ~1-2% of lower energy hits/towers/clusters by about 1 GeV.

I'd like to ask, why do we keep these hits? Can we remove all hcal hits with energy below 10 MeV ?

@abdoulline @igv4321

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes or unless it breaks tests.

davidlange6 added a commit that referenced this pull request Jan 6, 2015
@davidlange6 davidlange6 merged commit 1e18711 into cms-sw:CMSSW_7_4_X Jan 6, 2015
@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2015

@abdoulline
Copy link

Good point about very small RecHits, Salava.
All downstream consumers used to noremally impose much higher cuts.

There is one exlcusion, however - NZS stream, where we read out all the
cells without suppression.
So we need:
(1) to make sure introduction of "direct" (RecHit level) cut of ~100 MeV
cut will not affect anything
(2) to make sure it's switched off in case of NZS

On Tue, 6 Jan 2015, Slava Krutelyov wrote:

+1

for #7015 3a8cd2e
tested in CMSSW_7_4_X_2014-12-30-1400 (test area sign482)

The pedestal constraint part was indeed missed. Thanks for the fix.

Changes are generally small and become more visible in samples with pileup.
Here are some plots from 25202 (ttbar PU35@25ns)

all_sign482vsorig_ttbarpuwf25202p0c_log10hbherechitssorted_hbhereco__reco_obj_obj_energy
this propagates to caloTowers and other quantities as migration of ~1-2% of lower energy hits/towers/clusters by
about 1 GeV.

I'd like to ask, why do we keep these hits? Can we remove all hcal hits with energy below 10 MeV ?

@abdoulline @igv4321


Reply to this email directly or view it on GitHub.[AEx02p5SKUgnJ8C5XuzkXEsdKP9cz8ZFks5nfCd1gaJpZM4DMlLT.gif]

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2015

@slava77
Copy link
Contributor

slava77 commented Jan 6, 2015

Hi Salavat, I posted my re on HN
https://hypernews.cern.ch/HyperNews/CMS/get/hcal-cmssw/15.html

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