DM-26629: update to use calibration collections #54
Conversation
This removes the need for all "*Proposal" dataset types, and massively simplifies certification, because that's now a registry-only operation that doesn't involve any other I/O.
These tasks are trying to do something ill-defined according to our data model: get a camera with no temporal information. This workaround should work for now, but we need a better solution, and I'm not sure if that's a change to the tasks or a change to the data model.
bin.src/certifyCalibration.py
Outdated
| inputCollection=args.inputCollection, | ||
| outputCollection=args.outputCollection) | ||
| outputCollection=args.outputCollection, | ||
| lastRunOnly=not args.lastRunOnly) |
There was a problem hiding this comment.
This is a bit confusing, but I see it's because of the user-facing option and the CertifyCalibration option having different meanings.
There was a problem hiding this comment.
You were right to be confused; it's wrong, and it's how I broke this script (I added the command-line option after I tested ci_cpp_gen3 with the value defaulted to True in the task). Fixing it now.
| surprising by users (but will usually go unnoticed because the dataset | ||
| returned _is_ actually in those given input colllections, too). It may | ||
| stop working entirely once we have data repositories with multiple | ||
| calibration collections; a better workaround or a more principled change |
There was a problem hiding this comment.
This implies that multiple calibration collections are not allowed? And that writeCuratedCalibrations puts those calibrations into a RUN collection? Does that mean they need to be certified into the calibration collection?
Typo on line 5: "colllections".
There was a problem hiding this comment.
Multiple calibration collections are allowed, but we lack both naming conventions and a good workflow for populating more than one with curated calibrations, and this function essentially relies on those inadequate naming conventions.
More precisely, writeCuratedCalibrations first puts to multiple RUN collections (one for each calibDate + one for all unbounded datasets) and then certifies them all into one CALIBRATION collection. It has a suffix parameter for the collection names, to make it possible to write to a totally distinct suite of collections each time it is invoked, but nothing changes the suffix right now (see the comment from Tim that I pinged you on in obs_base). This workaround will be fine as long writeCuratedCalibrations has been invoked with no suffix argument on the repo, and it will do the right thing as long as this is actually the version of the curated calibrations the user wants - it'll pick up the version created with no suffix regardless.
And, of course, it will only work as long as the camera's validity range actually is unbounded.
Will squash.
|
The certifyCalibration script does not match the current approach for scripts. It should be minimalist (a couple of lines) and have the actual content inside the module (conventially in lsst.cp.cpp.script or somewhere) where you can pull the argument parser out in a separate function so that the sphinx documentation can include it. I realize that it was already wrong before this ticket. |
No description provided.