HCAL 2017 HF TP dual-anode readout fixes. #16464
Conversation
The previous iteration used slides as a base for the code, using divisions for adjusting for the dual-anode read-out. Use multiplications instead, as in the uHTR spec and add an omitted shift for the right LSB.
|
A new Pull Request was created by @matz-e (Matthias Wolf) for CMSSW_8_1_X. It involves the following packages: SimCalorimetry/HcalTrigPrimAlgos @cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
|
||
| auto sum = long_fiber_val + short_fiber_val; | ||
| // If both towers are valid, we cut the sum in half | ||
| if (long_fiber_count > 0 and short_fiber_count > 0) | ||
| sum *= 0.5; | ||
| if (long_fiber_count == 0 or short_fiber_count == 0) |
There was a problem hiding this comment.
hi @matz-e - this logic is very cryptic for me - is there a more concise (and less-codey) summary we can refer to?
There was a problem hiding this comment.
I should update the comment (was focused on logic) to read
// If one of the fibers is invalid, use the value of the other one instead by doubling the sumOther than that, I can also link to the uHTR spec, which is a bit more verbose and covers a lot more.
Would that be better?
There was a problem hiding this comment.
@davidlange6 is that comment acceptable?
There was a problem hiding this comment.
@davidlange6 Just updated this with a fairly (IMO) verbose comment on the treatment of the energy calculations.
|
+1 |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
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, @smuzaffar |
cmsbuild
added
comparison-pending
l1-pending
pending-signatures
tests-pending
and removed
l1-approved
tests-approved
labels
|
Pull request #16464 was updated. @cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please check and sign again. |
Fix the energy calculation to match firmware. Duplicate valid energies when one part of the readout fails, and shift over the lower energy bits properly.