DM-23778: Add gen2 to 3 repo conversion #189
Conversation
| @@ -58,7 +58,7 @@ exposures: | |||
| deepCoadd_psfMatchedWarp: | |||
| template: deepCoadd/%(filter)s/%(tract)d/%(patch)s/psfMatchedWarp-%(filter)s-%(tract)d-%(patch)s-%(expId)d.fits | |||
| dcrCoadd_directWarp: | |||
| template: dcrCoadd/%(filter)s%(subfilter)s/%(tract)d/%(patch)s/warp-%(filter)s%(subfilter)s-%(tract)d-%(patch)s-%(expId)d.fits | |||
| template: dcrCoadd/%(filter)s%(subfilter)d/%(tract)d/%(patch)s/warp-%(filter)s%(subfilter)d-%(tract)d-%(patch)s-%(expId)d.fits | |||
There was a problem hiding this comment.
I didn't realize subfilters were always numbers. @isullivan , can you confirm?
There was a problem hiding this comment.
Hmm. Okay -- I was getting inconsistency errors from the conversion tool because other templates in obs_base (I think) define it as a number and not a string. Now that I look in daf_butler subfilter has to be a string so does that mean I need to fix obs_base?
There was a problem hiding this comment.
To be explicit here. obs_base uses:
%(filter)s%(subfilter)dof%(numSubfilters)d/
obs_decam uses:
%(filter)s%(subfilter)d
obs_lsst uses %(filter)s%(subfilter)s and daf_butler defines subfilter to be a string. Since the conversion tool reads the templates from obs_base you get this error:
ValueError: Multiple types for key 'subfilter': <class 'str'> (from dcrCoadd/%(filter)s%(subfilter)s/%(tract)d/%(patch)s/warp-%(filter)s%(subfilter)s-%(tract)d-%(patch)s-%(expId)d.fits) vs. <class 'int'>.
So I'm not entirely sure how to proceed. What I did do was make all the obs packages consistent but that is not consistent with gen3. Do I merge what I've got and we resolve this on a new ticket?
There was a problem hiding this comment.
I don't know what the right fix is, so I guess I'd vote for changing as little as possible to get things working until we know more, but your guess is as good as mine on how best to accomplish that.
There was a problem hiding this comment.
Looks good, but I'm not sure I'm completely on board with moving too much of writeCuratedCalibrations to obs_base, as the last commit states as a goal. There may well be some logic that should be common (it seems like it), but I'm wary of making things more fragile by assuming that different things can be more similar than they really ought to be, especially before we have more examples of those things.
config/convertRepo.py
Outdated
| @@ -0,0 +1,12 @@ | |||
| # Config overrides for converting gen2 to gen3 repos. | |||
|
|
|||
| from lsst.obs.base.gen2to3 import Translator, AbstractToPhysicalFilterKeyHandler | |||
There was a problem hiding this comment.
This import is for side-effects?
There was a problem hiding this comment.
No, it's a copy and paste from obs_decam which added some explicit translators in the config itself.
| definition["storageClass"], | ||
| universe=butler.registry.dimensions) | ||
| butler.registry.registerDatasetType(datasetType) | ||
| self._write_obs_lsst_data(butler, datasetType) |
There was a problem hiding this comment.
I'm not really convinced that making this code generic over dataset types is the right call; smells a little like incidental duplication. But I guess we'll see when the third dataset type is added here: if that requires a change to the generic version, we should just split it into separate blocks for each dataset type to let them do their own thing.
There was a problem hiding this comment.
Remember that obs_*_data packages are by definition being set up in the same way such that the read_all function will return them in standard form for a given dataset type. If someone is using obs_*_data calibrations then this code will be the same in every obs package -- it's already the same between obs_subaru and obs_lsst and will be the same for obs_decam. Ideally obs_base would have a mode that scans the obs_*_data package for curated calibrations from a standardized set and the obs packages don't have to worry about _data at all. The reason ingestCuratedCalibrations works for gen2 is because of this standardization. The interesting bit is going to come when we have a curated calibration that has different gen3 dimensions.
This is in preparation for moving it to obs_base so it can be reused by other obs packages.
No description provided.