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

Package version checking is non-deterministic

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: base
    • Labels:
      None

      Description

      When a command-line task runs, it writes the versions of packages currently set up to the repository. We're then unable to run the same task again without matching versions. This helps with reproducibility.

      However, when I repeatedly try to run the same task with exactly the same task with exactly the same packages set up, this version checking fails intermittently. The task exits, complaining:

      Traceback (most recent call last):
        File "/Users/jds/Projects/Astronomy/LSST/src/pipe_drivers/bin/constructBias.py", line 4, in <module>
          BiasTask.parseAndSubmit()
        File "/Users/jds/Projects/Astronomy/LSST/stack/DarwinX86/ctrl_pool/12.1-7-gb57f33e/python/lsst/ctrl/pool/parallel.py", line 422, in parseAndSubmit
          if not cls.RunnerClass(cls, batchArgs.parent).precall(batchArgs.parent):  # Write config, schema
        File "/Users/jds/Projects/Astronomy/LSST/stack/DarwinX86/pipe_base/12.1-5-g06c326c+6/python/lsst/pipe/base/cmdLineTask.py", line 303, in precall
          self._precallImpl(task, parsedCmd)
        File "/Users/jds/Projects/Astronomy/LSST/stack/DarwinX86/pipe_base/12.1-5-g06c326c+6/python/lsst/pipe/base/cmdLineTask.py", line 285, in _precallImpl
          task.writePackageVersions(parsedCmd.butler, clobber=parsedCmd.clobberVersions)
        File "/Users/jds/Projects/Astronomy/LSST/stack/DarwinX86/pipe_base/12.1-5-g06c326c+6/python/lsst/pipe/base/cmdLineTask.py", line 618, in writePackageVersions
          "); consider using --clobber-versions or --no-versions")
      lsst.pipe.base.task.TaskError: Version mismatch (meas_algorithms: 12.1-17-g13cfda1+6 with boost=1.60.lsst1+1 eigen=3.2.5.lsst2 vs 12.1-17-g13cfda1+6 with eigen=3.2.5.lsst2 boost=1.60.lsst1+1; coadd_utils: 12.1-1-g5961e7a+70 with boost=1.60.lsst1+1 eigen=3.2.5.lsst2 vs 12.1-1-g5961e7a+70 with eigen=3.2.5.lsst2 boost=1.60.lsst1+1; afw: 12.1-31-gb5bd9ab+1 with boost=1.60.lsst1+1 eigen=3.2.5.lsst2 vs 12.1-31-gb5bd9ab+1 with eigen=3.2.5.lsst2 boost=1.60.lsst1+1); consider using --clobber-versions or --no-versions
      

      Note that the versions are the same, but are being reported in a different order (12.1-17-g13cfda1+6 with boost=1.60.lsst1+1 eigen=3.2.5.lsst2 as compared to 12.1-17-g13cfda1+6 with eigen=3.2.5.lsst2 boost=1.60.lsst1+1).

      Please fix this so that version checking works reliably.

        Attachments

          Activity

          Hide
          swinbank John Swinbank added a comment -

          I'm intending to go ahead and fix this myself (it's easy enough when you figure out the trick), but I'm copying Fritz Mueller since I guess it's fundamentally a task framework issue.

          Show
          swinbank John Swinbank added a comment - I'm intending to go ahead and fix this myself (it's easy enough when you figure out the trick), but I'm copying Fritz Mueller since I guess it's fundamentally a task framework issue.
          Hide
          swinbank John Swinbank added a comment -

          The version comparison is performed by simply comparing the strings in the issue description for equality; no wonder there's a version mismatch when the ordering within the string changes. The strings are generated by reading from a set, so the ordering is non-deterministic.

          The order of iteration over a set is never guaranteed to be reproducible; it's likely this hasn't bitten us before because, by chance, it is in Python 2, but now we're discovering that it isn't in Python 3.

          Show
          swinbank John Swinbank added a comment - The version comparison is performed by simply comparing the strings in the issue description for equality ; no wonder there's a version mismatch when the ordering within the string changes. The strings are generated by reading from a set , so the ordering is non-deterministic. The order of iteration over a set is never guaranteed to be reproducible; it's likely this hasn't bitten us before because, by chance, it is in Python 2, but now we're discovering that it isn't in Python 3.
          Hide
          swinbank John Swinbank added a comment -

          Hey Paul Price, could I bother you for a review please? The change is very tiny!

          Jenkins jobs are still running; I'm fairly confident they'll be fine, but obviously I'll check and fix before merging if there's a problem.

          Show
          swinbank John Swinbank added a comment - Hey Paul Price , could I bother you for a review please? The change is very tiny! Jenkins jobs are still running; I'm fairly confident they'll be fine, but obviously I'll check and fix before merging if there's a problem. lsst_distrib lsst_py3
          Hide
          price Paul Price added a comment -

          Good fix. I'm sorry you had to go to trouble to fix something where I should have known better.

          Show
          price Paul Price added a comment - Good fix. I'm sorry you had to go to trouble to fix something where I should have known better.
          Hide
          swinbank John Swinbank added a comment -

          (Replying to your comment here, to avoid spamming all the GitHub watchers) —

          I slightly disagree about the placing of binary operators, based on the text in PEP-8. If it winds you up, I'm happy to change it, but if you don't object I'd probably rather leave it as-is.

          I think our coding standards are silent on this, but obviously if they agree with you then I immediately concede the point!

          Show
          swinbank John Swinbank added a comment - (Replying to your comment here, to avoid spamming all the GitHub watchers) — I slightly disagree about the placing of binary operators, based on the text in PEP-8 . If it winds you up, I'm happy to change it, but if you don't object I'd probably rather leave it as-is. I think our coding standards are silent on this, but obviously if they agree with you then I immediately concede the point!
          Hide
          swinbank John Swinbank added a comment -

          Confirmed with Paul that this is ok to merge as is. So I did.

          Show
          swinbank John Swinbank added a comment - Confirmed with Paul that this is ok to merge as is. So I did.

            People

            Assignee:
            swinbank John Swinbank
            Reporter:
            swinbank John Swinbank
            Reviewers:
            Paul Price
            Watchers:
            Fritz Mueller, John Swinbank, Paul Price
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: