Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-37559

DM-35207 broke ap_verify

    XMLWordPrintable

    Details

      Description

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

        Attachments

          Issue Links

            Activity

            Hide
            jbosch 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
            jbosch 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
            jbosch 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
            jbosch 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
            jbosch 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
            jbosch 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
            krzys Krzysztof Findeisen added a comment - - edited

            Done. Sorry for not catching the ConversionsForFakes omission!

            Show
            krzys Krzysztof Findeisen added a comment - - edited Done. Sorry for not catching the ConversionsForFakes omission!
            Hide
            ktl 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
            ktl 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
            jbosch 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
            jbosch 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:
              jbosch Jim Bosch
              Reporter:
              ktl Kian-Tat Lim
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Jim Bosch, Kian-Tat Lim, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.