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

HGCal "EM Scale" Calibrations #14877

Merged
merged 3 commits into from Jun 17, 2016
Merged

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Jun 14, 2016

Change RecHit producer to generate hits that are calibrated (roughly) to the electromagnetic shower scale to produce some "human readable" rec hit energies and to provide an estimate of cluster energies during cluster building.

@lgray
Copy link
Contributor Author

lgray commented Jun 14, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 14, 2016

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

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_8_1_X.

It involves the following packages:

RecoLocalCalo/HGCalRecAlgos
RecoLocalCalo/HGCalRecProducers

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @kpedro88, @argiro, @cseez, @pfs this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@lgray
Copy link
Contributor Author

lgray commented Jun 14, 2016

(FYI the unit test will fail since it is fixed in another PR)

@cmsbuild
Copy link
Contributor

-1

Tested at: 2738606

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14877/13502/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test runtestRecoLocalCaloHGCalRecProducers had ERRORS

@cmsbuild
Copy link
Contributor

break;
case HGCHEF:
rechitMaker_->setADCToGeVConstant(float(hgchefUncalib2GeV_) );
fCPerMIP = &HGCHEF_fCPerMIP_;
//fCPerMIP = &HGCHEF_fCPerMIP_;
Copy link
Contributor

Choose a reason for hiding this comment

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

better remove than comment out, unless there's a good reason to keep.
Add a comment why still needed.

@slava77
Copy link
Contributor

slava77 commented Jun 16, 2016

+1

for #14877 2738606

  • code change is in line with description
  • jenkins tests pass at compilation, the unit test failures are unrelated to this PR and are fixed in HGCal Digitizer update/bugfix: more accurate noise model #14872
  • locally checked with 11024.0 to take this as an opportunity to add some basic monitoring
    Energies change from something small to something larger but still not clearly physical in majority (dominated by noise?)
    all_sign729vsorig_ttbar13tev2023wf11024p0c_log10hgcrechitssorted_hgcalrechit_hgchefrechits_reco_obj_obj_energy
    all_sign729vsorig_ttbar13tev2023wf11024p0c_log10hgcrechitssorted_hgcalrechit_hgceerechits_reco_obj_obj_energy

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (but tests are reportedly failing). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6 davidlange6 merged commit 6e39fef into cms-sw:CMSSW_8_1_X Jun 17, 2016
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

4 participants