-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Remove dead assignments in RecoLocalMuon.CSCValidation.CSCValidation #31265
Remove dead assignments in RecoLocalMuon.CSCValidation.CSCValidation #31265
Conversation
The code-checks are being triggered in jenkins. |
float thisPedestal = 0.5 * (float)(myADCVals[0] + myADCVals[1]); | ||
float thisSignal = | ||
(1. / 6) * (myADCVals[2] + myADCVals[3] + myADCVals[4] + myADCVals[5] + myADCVals[6] + myADCVals[7]); | ||
float threshold = 13.3; | ||
if (id.station() == 1 && id.ring() == 4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ptcox I'm puzzled about this dead reassignment of myStrip
: was it intended to be somewhere else (e.g. above)?
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31265/17940
|
A new Pull Request was created by @perrotta for master. It involves the following packages: RecoLocalMuon/CSCValidation @perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Hi Andrea,
Thanks for drawing my attention to this.
It's completely beyond my comprehension too :)
However, it is pretty clear what it was once trying to do: in run 1 we
had the 48 strips of ME1/1A ganged every 3rd strip, so the 48 strips fed
into 16 channels.
In the offline, strip channels in ME1/1A were counted 1-16. (And now
they count 1-48).
But in the hardware, the count was tacked on after the 64 channels of
ME1/1B - so the software/real channel count 1-16 <-> a hardware/online
channel count 65-80. (This made some sense because in all other chamber
there are 80 channels, and so the count 1-80 is 'natural').
But WHY the code contains a reminder of this is beyond me. I presume it
was something introduced at some long ago time in the past before the
Unpacker decoded channels properly.
So I think there is no issues in suppressing it.
But thanks for noticing.
Regards, Tim
… perrotta ***@***.***>
August 27, 2020 at 17:41
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In RecoLocalMuon/CSCValidation/src/CSCValidation.cc
<#31265 (comment)>:
> float thisPedestal = 0.5 * (float)(myADCVals[0] + myADCVals[1]);
float thisSignal =
(1. / 6) * (myADCVals[2] + myADCVals[3] + myADCVals[4] + myADCVals[5]
+ myADCVals[6] + myADCVals[7]);
- float threshold = 13.3;
- if (id.station() == 1 && id.ring() == 4) {
@ptcox <https://github.com/ptcox> I'm puzzled about this dead
reassignment of |myStrip|: was it intended to be somewhere else (e.g.
above)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#31265 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGYLHT5I7BQMKVIXDQIENLSCZ5DFANCNFSM4QNEHAKA>.
|
Thank you @ptcox for your answer! |
+1
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
There were a few "Dead assignment" issues that popped out from the static analyzer in CSCValidation.
This PR intends to fix them (and apply also some further little cleaning)
PR validation:
It compiles
No changes expected