DM-46356: Update testing to use new IsrTaskLSST calibration pipeline #41
Conversation
There was a problem hiding this comment.
I think the small pipeline rearranging suggested would be helpful for the future (and to keep all pipeline constructions consistent), but that can be pushed to a separate ticket (since this is already in a mess of merges).
Same with the certify/verify command flips: I think we should do it, but a separate ticket would be fine to make sure the rest of the merge is smooth.
| [defects], | ||
| [ | ||
| getPipeTaskCmd("linearizer", exposureDict["ptcExposurePairs"], "cpLinearizer.yaml"), | ||
| getCertifyCmd("linearizer"), |
There was a problem hiding this comment.
Should the verify command run before the certify command? I see that the current code matches what's here, but when we run these for real, the verify is run against the uncertified calibration. It probably doesn't matter, but if we have connections that can only be met by certified calibrations, then that could cause problems in production. I'm happy to take any tickets this spawns if something is broken.
There was a problem hiding this comment.
I've added DM-46432 to look into this, so it doesn't need to complicate the merging here. That ticket will decide what the right solution is.
DATA/SConscript
Outdated
| inputCollections = f"ci_cpp_ptc,{inputCollections}" | ||
|
|
||
| pipelineYaml = os.path.join(PKG_ROOT, "pipelines", pipelineFile) | ||
| pipelineYaml = os.path.join(PKG_ROOT, "pipelines", f"LATISS_legacy_{legacyDate}", pipelineFile) |
There was a problem hiding this comment.
I think this should be "LATISS", f"legacy_{legacyDate}" so the ci_cpp_gen3 override pipelines match the paths of the cp_pipe and cp_verify legacy pipelines. This will require runIsrLSST.yaml to be moved into a LATISS subdirectory, but that will make extensions to different camera datasets easier in the future.
| @@ -1,7 +1,7 @@ | |||
| description: ci_cpp CROSSTALK calibration construction | |||
| instrument: lsst.obs.lsst.Latiss | |||
| imports: | |||
| - location: $CP_PIPE_DIR/pipelines/LATISS/cpCrosstalk.yaml | |||
| - location: $CP_PIPE_DIR/pipelines/LATISS/legacy_202409/cpCrosstalk.yaml | |||
There was a problem hiding this comment.
See comment above about adding subdirectories to have matching pipeline paths.
pipelines/runIsrLSST.yaml
Outdated
| class: lsst.ip.isr.IsrTaskLSST | ||
| config: | ||
| doBrighterFatter: false | ||
| doDeferredCharge: false |
There was a problem hiding this comment.
Mentioned above for instrument-specific pipeline path.
| @@ -1,16 +1,11 @@ | |||
| 2021052500015: | |||
| FAILURES: | |||
| - RXX_S00 C11 CR_NOISE | |||
| SUCCESS: true | |||
There was a problem hiding this comment.
This is reassuring that we're dealing with defects better now.
erykoff
force-pushed
the
tickets/DM-46356
branch
4 times, most recently
from
fc80b56 to
91d5e99
Compare
erykoff
force-pushed
the
tickets/DM-46356
branch
2 times, most recently
from
c902ab8 to
7dda3ed
Compare
Now setting CI_CPP_LEGACY environment variable to 1 will run the "legacy" pipelines and tests.
The BFK itself is still unverified and may not be good; see DM-46445.
erykoff
force-pushed
the
tickets/DM-46356
branch
from
7dda3ed to
7cfd201
Compare
erykoff
force-pushed
the
tickets/DM-46356
branch
from
7a224d7 to
2f15271
Compare
erykoff
force-pushed
the
tickets/DM-46356
branch
from
927cabe to
b7e7c02
Compare
The old "legacy" calibration pipelines can still be tested locally by setting the environment variable
export CI_CPP_LEGACY=1.