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

Stack demo failure with devtoolset-6

    Details

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

      Description

      Per DM-9955 (and confirmed by me on lsst-dev01), we see numerical values falling outside the expected tolerances when running the lsst_dm_stack_demo build with devtooset-6 (GCC 6.2.1).

      Please figure out what's going on and fix it.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment - - edited

            Is this investigation going to be scheduled this year?

            Show
            tjenness Tim Jenness added a comment - - edited Is this investigation going to be scheduled this year?
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Was caused by a combination of:

            1. Using single precision internally to calculate centroid errors, and
            2. Using a threshold == 0.0 by default in the demo comparison.

            Fixing the former (and regenerating the comparison tables) was sufficient to get the test to pass on both compiler versions, but probably we would like to fix the latter to some more sensible value too (at some later date).

            While I limited the current changes to the centroid error calculation, I can imagine we would want to examine other cases where using single precision for internal (accumulation) variables can cause problems in the centroiding code.

            I tested the fix on OSX and lsst-dev, but unfortunately, I cannot run CI because Jenkins ignores the branch for the demo (and always uses master).

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Was caused by a combination of: 1. Using single precision internally to calculate centroid errors, and 2. Using a threshold == 0.0 by default in the demo comparison. Fixing the former (and regenerating the comparison tables) was sufficient to get the test to pass on both compiler versions, but probably we would like to fix the latter to some more sensible value too (at some later date). While I limited the current changes to the centroid error calculation, I can imagine we would want to examine other cases where using single precision for internal (accumulation) variables can cause problems in the centroiding code. I tested the fix on OSX and lsst-dev, but unfortunately, I cannot run CI because Jenkins ignores the branch for the demo (and always uses master).
            Hide
            swinbank John Swinbank added a comment - - edited

            Thanks for tracking this down!

            I think it'd be helpful to add a bit more commentary in the commit message about exactly what's motivating this change.

            Also, are there performance implications?

            Show
            swinbank John Swinbank added a comment - - edited Thanks for tracking this down! I think it'd be helpful to add a bit more commentary in the commit message about exactly what's motivating this change. Also, are there performance implications?
            Hide
            swinbank John Swinbank added a comment -

            Relevant ticket for getting Jenkins to use the appropriate lsst_dm_stack_demo branch is DM-1357. However, if you have tested manually you can go ahead and merge — just stand by to bail Jenkins out if it breaks!

            Show
            swinbank John Swinbank added a comment - Relevant ticket for getting Jenkins to use the appropriate lsst_dm_stack_demo branch is DM-1357 . However, if you have tested manually you can go ahead and merge — just stand by to bail Jenkins out if it breaks!
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Merged. Did a quick performance test on the demo and didn't see a noticeable difference.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Merged. Did a quick performance test on the demo and didn't see a noticeable difference.

              People

              • Assignee:
                pschella Pim Schellart [X] (Inactive)
                Reporter:
                swinbank John Swinbank
                Reviewers:
                John Swinbank
                Watchers:
                John Swinbank, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Tim Jenness
              • Votes:
                1 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel