DM-30130: Establish a 1-1 correspondence between exposures and input dimensions in cpPtcExtract #89
Conversation
| # and inputs['inputDims']. This will be used in `run` when saving partial datasets. | ||
| expIdToInputDim = {} | ||
| for exp, dim in zip(inputs['inputExp'], inputs['inputDims']): | ||
| expId = exp.getInfo().getVisitInfo().getExposureId() |
There was a problem hiding this comment.
I think we want to get away from using getInfo().getVisitInfo().getExposureId(). Is it possible to just pass the inputDims to the arrange functions, and have them be sorted as well? This would future-proof this part of the code in case the info/VisitInfo objects change. This would also remove the need to pass a third mapping object to the run method.
There was a problem hiding this comment.
The DataId will have the detector number and exposure ID in it already. This is not the same thing as getExposureId because getExposureId is the detector exposure ID not the exposure ID. We are actively discussing changing getExposureId so it would be best not to use it here. I think for grouping you are better off creating a tuple of exposure+detector -- then you are being explicit in how you are doing your grouping using gen3 concepts and not relying on the visitInfo at all.
There was a problem hiding this comment.
If I'm not clear, I mean that you use the tuple of the exposure and detector dataId values as the key in your grouping algorithm.
| # Below, np.where(expId1 == np.array(inputDims)) (and the other analogous | ||
| # comparisons) returns a tuple with a single-element array, so [0][0] | ||
| # Below, np.where(expId1 == np.array(inputDims)) returns a tuple | ||
| # with a single-element array, so [0][0] |
There was a problem hiding this comment.
This logic would also be replaced by simply pulling the exposure values at the same time as the exposures in the loop above.
| outputs = self.run(**inputs) | ||
| butlerQC.put(outputs, outputRefs) | ||
|
|
||
| def run(self, inputExp, inputDims): | ||
| def run(self, inputDims, inputExp): |
There was a problem hiding this comment.
This switch seems unnecessary now.
No description provided.