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

HBHE M3 = fix M3 (90) #17984

Merged
merged 2 commits into from Mar 21, 2017
Merged

Conversation

mariadalfonso
Copy link
Contributor

backport of the #17983

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mariadalfonso for CMSSW_9_0_X.

It involves the following packages:

CalibCalorimetry/HcalAlgos

@ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @tocheng this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@slava77
Copy link
Contributor

slava77 commented Mar 19, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 19, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18549/console Started: 2017/03/19 13:11

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@mariadalfonso
Copy link
Contributor Author

@slava77 @fwyzard @deguio
it's important to have this PR in the 90X so that the siPM can be properly calibrated

@fwyzard
Copy link
Contributor

fwyzard commented Mar 21, 2017

sigh, this might be late for 9.0.0 ?

@mariadalfonso , does the change affect DIGI or HLT/RECO ?
In other words, if we run with a release that contains this PR (e.g. 9.1.x) over MC samples made with a release without this PR (e.g. 8.3.0) do we get the old or new results ?

@mariadalfonso
Copy link
Contributor Author

@fwyzard
This PR doesn't change the DIGI and you need to check with @kpedro88 if that is ok for the HCAL-DIGI point of view.

Regarding the RECO/HLT
https://github.com/cms-sw/cmssw/releases/tag/CMSSW_8_3_0
the 8.3.0 should not be use neither for the offline PF calibration neither for the online one
as it miss the M2 changes (#17533 #17311) and M3 changes (#17984 #17871)
of course is ok if you use the 9.1.x to re-HLT/re-RECO

@franzoni
Copy link

attn: @slava77

There's something inaccurate in the fact that this PR has alca signature - stemming from the fact that CalibCalorimetry/HcalAlgos holds both calibration related code and local reco / shape modelling classes.
@deguio @hatakeyamak would it be possible to factor our into reco or sim packages code which pertains there?

@franzoni franzoni mentioned this pull request Mar 21, 2017
@slava77
Copy link
Contributor

slava77 commented Mar 21, 2017 via email

@kpedro88
Copy link
Contributor

@franzoni the problem is that many of these classes need to be used by both SIM and RECO. The guidelines for what packages should host such classes are not very clear.

@fwyzard
Copy link
Contributor

fwyzard commented Mar 21, 2017

+1

@fwyzard
Copy link
Contributor

fwyzard commented Mar 21, 2017

(signature for psychological support)

@cmsbuild
Copy link
Contributor

Pull request #17984 was updated. @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please check and sign again.

@mariadalfonso
Copy link
Contributor Author

fixed array based to the discussion in #17983

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 21, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18608/console Started: 2017/03/21 20:48

@davidlange6 davidlange6 merged commit d56a536 into cms-sw:CMSSW_9_0_X Mar 21, 2017
@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Mar 22, 2017

backport of #17983

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

7 participants