DM-47535: Add unit test for reprocessVisitImage and enable useCiLimits on its deblending. #44
Conversation
TallJimbo
force-pushed
the
tickets/DM-47535
branch
from
65067e7 to
2c46164
Compare
| @@ -92,6 +92,7 @@ def run(self, currentState: BuildState): | |||
| "--skip-existing", | |||
| "--save-qgraph", os.path.join(self.runner.RunDir, QGRAPH_FILE), | |||
| "--config", f"calibrate:deblend.useCiLimits={not self.arguments.no_limit_deblend}", | |||
| "--config", f"reprocessVisitImage:deblend.useCiLimits={not self.arguments.no_limit_deblend}", | |||
There was a problem hiding this comment.
Do we really need this? I removed the calibrate line above, and I'm not sure it's really necessary here. Maybe it is necessary for reprocessVisitImage?
EDIT: oh, I see I did add it for reprocessVisitImage on #43 . I'm still not sure it's really necessary, but I guess it's not worth quibbling over.
There was a problem hiding this comment.
Yeah, I'm not sure how much faster it makes ci_imsim, but I assume it was added for a good reason initially and I imagine that reasoning would apply to reprocessVisitImage as well.
| self.exposure = self.butler.get("pvi", self.dataId) | ||
| self.catalog = self.butler.get("sources_footprints_detector", self.dataId) | ||
|
|
||
| def testLocalPhotoCalibColumns(self): |
There was a problem hiding this comment.
I do hope that we can remove LocalPhotoCalib from the catalogs in the future, since they'll have calibrated values. Not sure if that's ticketed?
There was a problem hiding this comment.
The transform*Table catalog implementations are pandas-heavy and one of the bits of the stack I'm least familiar with, and that makes me really not want to wade in to them as long as they're working. So if we ever do fix this, I think it'll be because we decide to do a full rewrite of that stuff, and that won't benefit much from a more targeted ticket.
Previously, no_limit_deblend gave us the option to turn off useCiLimits for CalibrateTask. reprocessVisitImage is the new calibrate with useCiLimits=True in the ci_imsim pipeline by default
TallJimbo
force-pushed
the
tickets/DM-47535
branch
from
2c46164 to
ed0aaee
Compare
No description provided.