Using aux word to trace the origin of combined Plan 1 rechits

[igv4321]

Added methods needed for getting Plan 1 workflow to work (requested by Kevin and Sunanda).

This PR will not modify any existing results. Please merge asap.

[cmsbuild]

A new Pull Request was created by @igv4321 (Igor Volobouev) for CMSSW_9_0_X.

It involves the following packages:

DataFormats/HcalRecHit
RecoLocalCalo/HcalRecAlgos

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

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17795/console Started: 2017/02/15 13:54

[cmsbuild]

+1
Tested at: 32f8cb9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17516/17795/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17516/17795/summary.html

[slava77]

some protection should be added in PFHBHERecHitCreatorMaxSample (method 1.5)
https://github.com/cms-sw/cmssw/blob/CMSSW_9_0_0_pre4/RecoParticleFlow/PFClusterProducer/interface/PFHBHERecHitCreatorMaxSample.h#L78
maybe just an assert that checks for OFF_COMBINED flag

[igv4321]

Since we are switching to Phase 1 reconstruction anyway, that particular code will have to be reworked in a much more radical manner. The Phase 1 ADC packing scheme is different since it has to accommodate 8-bit values. Anyway, this issue should be noted but it is not really related to this PR.

[slava77]

On 2/15/17 11:41 AM, Igor Volobouev wrote: Since we are switching to Phase 1 reconstruction anyway, that particular code will have to be reworked in a much more radical manner. The Phase 1 ADC packing scheme is different since it has to accommodate 8-bit values. Anyway, this issue should be noted but it is not really related to this PR.

Do you mean that HPD hits in plan-1 reco have aux content different from what's in phase-0 reconstruction of HPD/QIE8 in scenarios up to 2016? If plan-1 hits for HPD channels remain in the same format, then PFHBHERecHitCreatorMaxSample still works and a protection will be needed for it to do something else or skip the SiPM/QIE11 hits. It can be made in a separate PR.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17516 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbkUKqPkbZxotJLseGMqmR8UjoaGXks5rc1TvgaJpZM4MBXNf>.

[igv4321]

Yes, they are different, and HPD channels will not remain in the same format. Naturally, we wanted to have the same packing/unpacking code for Phase 1 HB and HE. Here is the scheme which defines the Phase 1 ADC packing:
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecAlgos/src/HBHERecHitAuxSetter.cc
ADC values are packed into bytes (instead of 7-bit pieces), and there is a new aux word as well.

[slava77]

Thanks for the reminder of the switch to 8 bits.

[slava77]

+1

for #17516 32f8cb9

• code changes are in line with the description; existing workflows are not affected; the plan-1 workflow will be affected in a somewhat technical way (there are no consumers of the aux fields at this point)
• jenkins tests pass and comparisons with baseline show no differences

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

[davidlange6]

+1

[smuzaffar]

@igv4321 , this breaks fwlite build. Can we avoid this dependency?

src/DataFormats/HcalRecHit/src/HBHERecHit.cc:3:70: fatal error: RecoLocalCalo/HcalRecAlgos/interface/HBHERecHitAuxSetter.h: No such file or directory

[slava77]

Can we get a check in jenkins tests for unwanted dependencies?

[davidlange6]

ah- yes, we can not have this dependency.

…

On Feb 16, 2017, at 11:32 AM, Malik Shahzad Muzaffar ***@***.***> wrote: @smuzaffar commented on this pull request. In DataFormats/HcalRecHit/src/HBHERecHit.cc: > @@ -1,5 +1,7 @@ #include "DataFormats/HcalRecHit/interface/HBHERecHit.h" +#include "RecoLocalCalo/HcalRecAlgos/interface/HBHERecHitAuxSetter.h" +#include "RecoLocalCalo/HcalRecAlgos/interface/CaloRecHitAuxSetter.h" @igv4321 , this breaks fwlite build. Can we avoid this dependency? src/DataFormats/HcalRecHit/src/HBHERecHit.cc:3:70: fatal error: RecoLocalCalo/HcalRecAlgos/interface/HBHERecHitAuxSetter.h: No such file or directory — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[igv4321]

Hm... This was a header dependency only, not a library dependency...
Anyway, fixed in PR #17534

[davidlange6]

no matter…data formats are meant to depend on nothing Thanks

…

On Feb 16, 2017, at 12:38 PM, Igor Volobouev ***@***.***> wrote: Hm... This was a header dependency only, not a library dependency... Anyway, fixed in PR #17534 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.