Fix Version/s: None
Sprint:DRP F16-1, DRP F16-2
Team:Data Release Production
Adding a butler argument to the constructor signatures for CharacterizeImageTask, CalibrateTask, and ProcessCcdTask makes these tasks difficult to use without a butler.
The fix is to make the butler argument optional (with a default of None), while adding another argument that allows a fully-constructed reference object loader to be provided directly instead.
This is closely related to
DM-6597, which has the opposite problem: pipe_drivers' SingleFrameDriverTask doesn't take a butler argument, but it needs to in order to provide one to ProcessCcdTask.
I have a fix for this just about ready, but I'd like to add some unit tests that verify we can run all of these tasks both from the command-line and directly before calling it complete.
- is blocked by
DM-6641 Add exact variants of image comparison test utilities
This ticket makes it possible to run the immediate subtasks of ProcessCcdTask IsrTask, CharacterizeImageTask, and CalibrateTask) both directly from the command-line and from Python without access to the butler. The changes to the tasks themselves are small - mostly adding construcor arguments and making other arguments optional. Most of the new code is test code in pipe_tasks, but there are also changes to the camera mappers to provide config/metadata entries for these tasks (which are necessary to run them directly from the command line).
Feel free to create any GitHub PRs that would be helpful in the review; I'm not going to create them all in advance because the changes in some packages are sufficiently trivial that a PR may not be necessary.
- A question about passing refObjLoader from SingleFrameDriver to ProcessCcdTask down to CharacterizeImageTask and CalibrateTask, noted on PR.
- About the mapper policy paf files in obs packages:
obs_cfht, obs_lsstSim, obs_sdss, obs_test: Isn't raw_skyTile no longer used?
obs_decam: processCcd_config and processCcd_metadata are repeated
- Currently the config/processCcd.py files in most obs packages include config for the three subtasks. Does it make sense to split the processCcd configuration into separate files for all obs packages? Or we leave that for each obs pacakge to do later?
- Now IsrTask is a usable command line task (thank you!), does it become the only CmdLineTask outside pipe_tasks? If so, should we move it to pipe_tasks now, or leave it for the future package reorginization plan? Also, do we care to add executables for the three tasks?
Command-line tasks are encouraged to be in the package that makes most sense for them, so IsrTask belongs in ip_isr. pipe_tasks is intended for tasks (command-line and otherwise) that need so many packages that they don't make sense being in a lower-level package
Hsin-Fang Chiang, all good questions.
I'm afraid I don't know anything about raw_skyTile's status - I was just copying existing paf entries and modifying them, so I'm just inheriting an existing problem. If you can point me at an example of how I should remove any references to raw_skyTile, I'd be more than happy to fix that in the lines I've changed.
I'll fix the duplications in obs_decam.
I think we do want to split the processCcd configurations into separate files in the other obs packages, but I'd prefer not do do that on this ticket. I imagine that means I should create some new issues for that.
I second Russell Owen's reply on CmdLineTask lcoations; FWIW, there are also a few in in meas_base and (I think) ip_diffim.
Finally, I'm embarrassed I didn't notice the lake of executables for these Tasks. I'll certainly go ahead and add them. We may also want to encourage obs packages with their own ISR subclass to provide their own executables (I'll mention that in the issues I already promised to make). Eventually we'll need to solve the problem how how to run camera-retargeted tasks from the command-line in a generic way, but we'll need new task/config framework features to do that.
I think that a bin script to handle ISR is going to take some thought, because every camera overrides that task. It may need a parent task whose sole purpose is to call the retargetable IsrTask. Another option is a set of bin scripts, one per camera. I suggest it be a different ticket.
Russell Owen good point on ISR bin scripts. I'll defer that, and I think that means I should remove the isr_config and isr_metadata paf entries I've added for most cameras as well, given that they'll only enable something that shouldn't be run anyway.
UPDATED: Actually, I think I should just modify the isr_config entries to point to the right per-obs IsrTasks. I think that's an unambiguous step in the right direction, even if it doesn't enable anything on its own.
Agree, and thanks for pointing out meas_base. I think SuperTask work might want to know where all CmdLineTasks are.
Please add new common mapper entries in daf_butlerUtils rather than in every obs package.
Paul Price That sounds great, but do you have some docs or an example of how to do that?
Found the docs here: https://community.lsst.org/t/centrally-defined-butler-datasets/841 (I was confused by the fact that only datasets.yaml currently exists).
I've moved the _config entries (except isr_config) to daf_butlerUtils. The metadata entries and isr_config have to stay because they differ between obs packages.
Just waiting on final confirmation from Jenkins now.
I have just moved some reviewers to watcher status (I'd just put them in the wrong place to start with). Sorry about the confusion.