# DM-35207 broke ap_verify

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
0.5
• Team:
Data Release Production
• Urgent?:
No

#### Description

ap_verify is reporting lsst.pipe.base.pipeTools.DuplicateOutputError: DatasetType visitSummary' appears more than once as output.

#### Activity

Hide
Jim Bosch added a comment -

Huh.  I did run ap_verify locally, but on the ci_hits2015 dataset rather than the ci_cosmos_pdr2 dataset that seems to have failed here.  Maybe the pipelines are meaningfully different, or maybe there was some user error on my part.  In any case, I'm on it.

Show
Jim Bosch added a comment - Huh.  I did run ap_verify locally, but on the ci_hits2015 dataset rather than the ci_cosmos_pdr2 dataset that seems to have failed here.  Maybe the pipelines are meaningfully different, or maybe there was some user error on my part.  In any case, I'm on it.
Hide
Jim Bosch added a comment - - edited

Ok, comparing logs, it looks like the problem is in the fakes pipeline and apparently I wasn't testing that by just running

  ap_verify.py --dataset $DATASET I'm guessing the Jenkins job also runs something like:  ap_verify.py --dataset$DATASET --pipeline $DATASET_DIR/pipelinw/ApVerifyWithFakes.yaml ` I can reproduce the problem with that, at least, but if anyone can correct this or point me at how to figure out exactly what the Jenkins job invokes I'd appreciate it to make sure I've done the same. Show Jim Bosch added a comment - - edited Ok, comparing logs, it looks like the problem is in the fakes pipeline and apparently I wasn't testing that by just running ap_verify.py --dataset$DATASET I'm guessing the Jenkins job also runs something like: ap_verify.py --dataset $DATASET --pipeline$DATASET_DIR/pipelinw/ApVerifyWithFakes.yaml   I can reproduce the problem with that, at least, but if anyone can correct this or point me at how to figure out exactly what the Jenkins job invokes I'd appreciate it to make sure I've done the same.
Hide
Jim Bosch added a comment - - edited

Ok, I think this is fixed; if I'd taken the initiative to really exhaustively apply a review comment from Krzysztof Findeisen on DM-35207 to other similar spots, I might have avoided this entirely, but some of those were pretty subtle.

I made a choice here to leave my changes in pipe_tasks that broke things and undo them in ap_pipe and ap_verify, rather than reverting those changes in pipe_tasks and updating drp_pipe to reapply them there, for three reasons:

• it's now the smaller change (even if it'd have been a larger change relative to the codebase prior to DM-35207);
• it saves me from having to re-run the slower DRP integration tests before merging this, since I know changes in ap_pipe and ap_verify can't break those;
• I'm planning to propose further changes to the AP fakes pipeline to bring it more in line with the DRP one on RFC-901, so my best guess right now is that this is also the more future-proof choice.

Krzysztof Findeisen, could you review? PRs are:

Show
Jim Bosch added a comment - - edited Ok, I think this is fixed; if I'd taken the initiative to really exhaustively apply a review comment from Krzysztof Findeisen on DM-35207 to other similar spots, I might have avoided this entirely, but some of those were pretty subtle. I made a choice here to leave my changes in pipe_tasks that broke things and undo them in ap_pipe and ap_verify, rather than reverting those changes in pipe_tasks and updating drp_pipe to reapply them there, for three reasons: it's now the smaller change (even if it'd have been a larger change relative to the codebase prior to DM-35207 ); it saves me from having to re-run the slower DRP integration tests before merging this, since I know changes in ap_pipe and ap_verify can't break those; I'm planning to propose further changes to the AP fakes pipeline to bring it more in line with the DRP one on RFC-901 , so my best guess right now is that this is also the more future-proof choice. Krzysztof Findeisen , could you review? PRs are: https://github.com/lsst/ap_pipe/pull/128 https://github.com/lsst/ap_verify/pull/177
Hide
Krzysztof Findeisen added a comment - - edited

Done. Sorry for not catching the ConversionsForFakes omission!

Show
Krzysztof Findeisen added a comment - - edited Done. Sorry for not catching the ConversionsForFakes omission!
Hide
Kian-Tat Lim added a comment -

Sorry this is late, but, for the record, the Jenkins job invokes pipelines on datasets in accordance with https://github.com/lsst-dm/jenkins-dm-jobs/blob/main/etc/scipipe/ap_verify.yaml using run_ci_dataset.sh.

Show
Kian-Tat Lim added a comment - Sorry this is late, but, for the record, the Jenkins job invokes pipelines on datasets in accordance with https://github.com/lsst-dm/jenkins-dm-jobs/blob/main/etc/scipipe/ap_verify.yaml using run_ci_dataset.sh .
Hide
Jim Bosch added a comment -

Backports to v25 and v24 complete. Backport to v24 was only ap_pipe, as the changes made by DM-35207 to ap_verify were on top of changes that weren't present in v24.

Show
Jim Bosch added a comment - Backports to v25 and v24 complete. Backport to v24 was only ap_pipe, as the changes made by DM-35207 to ap_verify were on top of changes that weren't present in v24.

#### People

Assignee:
Jim Bosch
Reporter:
Kian-Tat Lim
Reviewers:
Krzysztof Findeisen
Watchers:
Jim Bosch, Kian-Tat Lim, Krzysztof Findeisen