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

lsst_ci fails with astropy 4 and numpy >=1.17

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: lsst_ci
    • Labels:
      None
    • Story Points:
      1
    • Team:
      External
    • Urgent?:
      No

      Description

      In testing DM-22817 I discovered that lsst_ci doesn't like Astropy 4 because it seems that Astropy with numpy >=1.17 now preserves units inside numpy functions: (resulting in us applying the unit twice):

      /Users/square/j/ws/stack-os-matrix/osx.clang.py3/lsstsw/stack/DarwinX86/validate_drp/19.0.0-7-g12b746e+17/python/lsst/validate/drp/photerrmodel.py:71: RuntimeWarning: invalid value encountered in sqrt
      return np.sqrt(sigmaSq)
      Traceback (most recent call last):
      File "/Users/square/j/ws/stack-os-matrix/osx.clang.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.7/site-packages/astropy/units/quantity_helper/helpers.py", line 32, in get_converter
      scale = from_unit._to(to_unit)
      File "/Users/square/j/ws/stack-os-matrix/osx.clang.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.7/site-packages/astropy/units/core.py", line 951, in _to
      f"'{self!r}' is not a scaled version of '{other!r}'")
      astropy.units.core.UnitConversionError: 'Unit("marcsec")' is not a scaled version of 'Unit("marcsec2")'
      

      (from https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/31182/tests/ )

      This fails for testObsCfhtQuick and testObsDecamQuick

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I'm happy to review this as soon as there is a pull request. I've started a Jenkins run.

            Show
            tjenness Tim Jenness added a comment - I'm happy to review this as soon as there is a pull request. I've started a Jenkins run.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Thanks, Tim Jenness for running the testing against AstroPy 3 and AstroPy4. That was the part that I figured would just take me an hour or two of being stupid before I got it to run right.

            I've created a pull request.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Thanks, Tim Jenness for running the testing against AstroPy 3 and AstroPy4. That was the part that I figured would just take me an hour or two of being stupid before I got it to run right. I've created a pull request.
            Hide
            tjenness Tim Jenness added a comment -

            The patch as it stands doesn't quite work. There was another place using np.percentile. With the following change things pass with new Astropy/numpy:

            diff --git a/python/lsst/validate/drp/calcsrd/pa2.py b/python/lsst/validate/drp/calcsrd/pa2.py
            index 6dd6c0a..8b98c47 100644
            --- a/python/lsst/validate/drp/calcsrd/pa2.py
            +++ b/python/lsst/validate/drp/calcsrd/pa2.py
            @@ -59,5 +59,5 @@ def measurePA2(metric, pa1, pf1_thresh):
                 magDiffs = pa1.extras['magDiff'].quantity[0, :]
             
                 pf1Percentile = 100.*u.percent - pf1_thresh
            -    return Measurement(metric, np.percentile(np.abs(magDiffs), pf1Percentile.value) * magDiffs.unit,
            +    return Measurement(metric, np.percentile(np.abs(magDiffs.value), pf1Percentile.value) * magDiffs.unit,
                                    extras=datums)
            

            Show
            tjenness Tim Jenness added a comment - The patch as it stands doesn't quite work. There was another place using np.percentile . With the following change things pass with new Astropy/numpy: diff --git a/python/lsst/validate/drp/calcsrd/pa2.py b/python/lsst/validate/drp/calcsrd/pa2.py index 6dd6c0a..8b98c47 100644 --- a/python/lsst/validate/drp/calcsrd/pa2.py +++ b/python/lsst/validate/drp/calcsrd/pa2.py @@ -59,5 +59,5 @@ def measurePA2(metric, pa1, pf1_thresh): magDiffs = pa1.extras['magDiff'].quantity[0, :] pf1Percentile = 100.*u.percent - pf1_thresh - return Measurement(metric, np.percentile(np.abs(magDiffs), pf1Percentile.value) * magDiffs.unit, + return Measurement(metric, np.percentile(np.abs(magDiffs.value), pf1Percentile.value) * magDiffs.unit, extras=datums)
            Hide
            tjenness Tim Jenness added a comment -

            Looks good to me. lsst_ci now passes with my updated conda packages. There is a jenkins run going but I imagine that won't cause any difficulties https://ci.lsst.codes/job/stack-os-matrix/31198/display/redirect

            Show
            tjenness Tim Jenness added a comment - Looks good to me. lsst_ci now passes with my updated conda packages. There is a jenkins run going but I imagine that won't cause any difficulties https://ci.lsst.codes/job/stack-os-matrix/31198/display/redirect
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Jenkins run finished clean for CentOS7. Unrelated error for OSX.

            Merged to master.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Jenkins run finished clean for CentOS7. Unrelated error for OSX. Merged to master.

              People

              • Assignee:
                wmwood-vasey Michael Wood-Vasey
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Tim Jenness
                Watchers:
                Michael Wood-Vasey, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel