| Simon | - Simon commenced the meeting with the bold statement: "We need to rewrite all of our tasks."
- Simon is reviewing a ticket from Dominique for his work to get CFHT running all the way through Coadd. He basically had to copy wholesale Isr task, Astrometry task, Meas_astrom, etc. There's a lot of duplication he had to do - to change probably 6 total lines of code. The problem is having actual code within the run methods instead of having the run method only call re-targetable subtasks. Simon thinks that if we were better about calling re-targetable subtasks in our run methods then Dominique's work would have been a lot easier.
- K-T asked for clarification of the re-targetable subtasks issue.
- Simon: A lot of our task code has code within the run methods. For example, the isr task:run contains config parameters which allow you to turn on and off different subtasks with booleans but the overscan task is not handled that way and is not a subtask. So, in order to turn off overscan correction, you have to copy isr's run method, delete that line, and then call the rest of the lines. If everything in the run method was re-targetable, you could reconfigure everything by targetting to a no-op or have config parameters turn it off. There may have been ways to do it with less copied code but there are probably 4 or 5 tasks that he had to do this with.
- Jim: Targetable subtasks aren't the only solution to that problem. You could also address those issues by delegating to other methods within the same class. You just need to make a trivial subclass within the main task.
- K-T: This shouldn't be a design discussion, I just wanted to understand some of the problems.
- Post meeting: Paul Price commented:
- As someone who frequently subclasses and overrides in order to support HSC, I have hit the same thing. I believe the problem is that some Task methods (often "run") are attempting to do too much, rather than breaking things into smaller pieces, which makes it difficult to subclass-and-override without copying large amounts of code. Besides refactoring existing instances as we find them, we also need to think as we code new modules what a user might possibly want to do and what functionalities he might want to override. This is difficult, but I think the guiding principle is that a method should only ever do one thing, and anything subordinate that it needs should be delegated to another method.
The HSC code doesn't copy the IsrTask wholesale; we have re-written the 'run' method, but that's mainly because we need to fit in additional functionality (e.g., amp crosstalk). I have, however, copied the AssembleCoaddTask wholesale in order to refactor it; I hope that this will make it back into LSST some day (see https://hsc-jira.astro.princeton.edu/jira/browse/HSC-617).
|