DM-32694: Split AP pipeline into ApPipeWithFakes #106
Conversation
|
I hope I answered your question about More broadly, are you OK with the fact that |
kfindeisen
force-pushed
the
tickets/DM-32694
branch
from
e2b0366
to
53c5d3a
Compare
|
I'm good with the restructuring you did to make the fakes pipeline stand alone - there are arguments to be made for having it import more from regular ApPipe, but I see why doing it this way makes ap_verify's life easier, and the labels are more clear and less likely to accidentally... I don't know... make a template with fakes and forget to use it 🙃 My only quibbles are with definitions of the subsets, as discussed above. If we can sort that out, AND if you're sure ap_verify (which imports the new ApPipeWithFakes pipelines) runs fine on both DECam and HSC datasets, then I'm quite happy. |
Explicitly defining a parameter for each possible dataset was too much of a hassle, so instead I rewrote the contracts to be template-agnostic.
Keeping the two pipelines separate makes it easier to enforce only the contracts that make sense for each pipeline. This is especially important for the fakes pipeline, where some configuration bugs may not get caught by the graph builder.
This prevents confusion between with-fakes and no-fakes versions of the same task, which happens in ap_verify.
ApVerifyWithFakes was using a different science exposure for difference imaging and forced photometry.
kfindeisen
force-pushed
the
tickets/DM-32694
branch
from
53c5d3a
to
f185107
Compare
The config line was added to the base pipeline on #106, but never synced to the instrument-specific pipelines.
This PR significantly reworks the AP pipelines, making two major changes:
ApPipeWithFakespipeline no longer importsApPipe, giving it independent task labels and contracts. This change is strictly for the benefit ofap_verify(which needs to be careful about distinguishing fakes and non-fakes tasks), and does not improveap_pipeitself.