DM-27854: CpFlatNormalizationTask improperly indexes detectors #68
Conversation
There was a problem hiding this comment.
The change looks OK and should fix the problem.
Please consider renaming detSet to, e.g., detectorsWithData and expSet correspondingly to, e.g., exposuresWithData. Throughout, I do think it would read more clearly if you wrote out exposure and detector in the variable names, including detectorScales and exposureScales, etc., but I'm not going to insist on it everywhere.
| # Note that the enumerated "detId"/"expId" here index the | ||
| # "detScales" and "expScales" arrays. |
There was a problem hiding this comment.
This comment does enhance clarity, but I'd prefer not reusing detId and expId here since they were defined previously (lines 254-255 or lines 273-274). Perhaps for detectorIdx, detector in enumerate(expSet) and for exposureIdx, exposure in enumerate(expSet)?
There was a problem hiding this comment.
I've changed every variable in the run method that is a matrix index to have the Idx suffix, to avoid confusion with dataId identifiers.
Use detector indices instead of detector-scale indices.