Initial implementation of the HB/HE Phase 1 reconstruction #15092
Conversation
|
A new Pull Request was created by @igv4321 (Igor Volobouev) for CMSSW_8_1_X. It involves the following packages: DataFormats/HcalRecHit @civanch, @cvuosalo, @mdhildreth, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
-1 Tested at: 6d2850b You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals
When I ran the RelVals I found an error in the following worklfows: DAS Error136.731 step1 DAS Error140.53 step1 DAS Error1000.0 step1 DAS Error1001.0 step1 DAS Error1003.0 step1 DAS Error |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
So, I felt "brave" and ran ttbar+PU35 based on Kevin's -m3 recipe in
PF buckled only in neutral hadrons without major change elewhere @kpedro88 are you using HCalCustoms.customise_Hcal2017Full for anything physics-like now? |
|
@slava77 no, we're not using any of the new/temporary HCAL customize functions for physics right now. They're just for testing/validation of individual components. (Once the new reco is finished, the module will be added to the run2_he_2017 Era, of course...) |
|
Slava, thank you for the plots. You are basically looking at the difference between "method 2" and "method 0" |
|
@civanch Could you see this PR and approve this? |
|
@civanch |
|
+1 |
cmsbuild
added
simulation-approved
fully-signed
and removed
pending-signatures
simulation-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar |
| /// get the id | ||
| HcalDetId id() const { return HcalDetId(detid()); } | ||
| inline HcalDetId id() const { return HcalDetId(detid()); } |
There was a problem hiding this comment.
@igv4321 do these two inline statements actually have an effect?
There was a problem hiding this comment.
There is only one inline statement, which replaced a non-inline
statement. This normally makes the code faster, bit it is still
up to the compiler whether to really inline. Since the body
is very short, there are no disadvantages to making this function
inline.
From: David Lange [notifications@github.com]
Sent: Thursday, July 14, 2016 1:48 AM
To: cms-sw/cmssw
Cc: Volobouev, I; Mention
Subject: Re: [cms-sw/cmssw] Initial implementation of the HB/HE Phase 1 reconstruction (#15092)
In DataFormats/HcalRecHit/interface/HBHERecHit.hhttps://github.com//pull/15092#discussion_r70756111:
/// get the id
- HcalDetId id() const { return HcalDetId(detid()); }
- inline HcalDetId id() const { return HcalDetId(detid()); }
@igv4321https://github.com/igv4321 do these two inline statements actually have an effect?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/15092/files/cb95544ae57dc7aa6cab5481015b80942b61d6d8#r70756111, or mute the threadhttps://github.com/notifications/unsubscribe/AFS4-WLGK48WoUxWabdz8kfb0yv5mr-9ks5qVdvPgaJpZM4JCc97.
|
It can be, though starting from a new ib, and cms-merge-topic-ing the original branch would allow this or to be modified trivially |
|
David, please merge. For a number of reasons, it is easier and more efficient to submit a new PR later. From: David Lange [notifications@github.com] It can be, though starting from a new ib, and cms-merge-topic-ing the original branch would allow this or to be modified trivially — |
|
+1 |
Includes the framework (CMSSW module, algorithms) for the HB/HE Phase 1 reconstruction
as well as a minor refactoring of a few HF-related functions. This code was discussed at
https://indico.cern.ch/event/546917/contributions/2218561/attachments/1301181/1943877/notes.pdf