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

AST persistence is not exact

    Details

    • Story Points:
      1
    • Sprint:
      Alert Production F17 - 10
    • Team:
      Alert Production

      Description

      AST persistence by writing to a string is not exact because double precision values are not written to sufficient precision. The result is tiny differences in computations from FrameSets that have been persisted and unpersisted. David Berry is working on a fix. This ticket provides a place to record the issue and mark temporary workarounds in our tests.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            David Berry fixed the problem in AST.

            I enhanced astshim as follows:

            • Added MappingTestCase.testMappingPersistence. This check that persistence is exact by transforming points using a mapping, and transforming points using a copy obtained by persisting and un-persisting the mapping. The test checks that the output is identical (not just close) in both cases. It checks forward and inverse, if available.
            • Enhanced ObjectTestCase.checkPersistence to test FitsChan (with encoding="Native", since that is the only encoding that supports all AST object types) as well as the existing Channel and XmlChan. Note that testMappingPersistence also tests all three channels.
            Show
            rowen Russell Owen added a comment - David Berry fixed the problem in AST. I enhanced astshim as follows: Added MappingTestCase.testMappingPersistence . This check that persistence is exact by transforming points using a mapping, and transforming points using a copy obtained by persisting and un-persisting the mapping. The test checks that the output is identical (not just close) in both cases. It checks forward and inverse, if available. Enhanced ObjectTestCase.checkPersistence to test FitsChan  (with encoding="Native", since that is the only encoding that supports all AST object types) as well as the existing Channel and XmlChan . Note that testMappingPersistence also tests all three channels.
            Hide
            rowen Russell Owen added a comment -

            Note: after issuing "rebuild starlink_ast" I found an updated "component.xml" file that incremented the version from 8.5 to 8.5.1. I committed it, but I'm wondering if should be omitted from the github repo (e.g. if it's automatically created). The version of AST is indeed 8.5.1, and some changes have been made since that version (including the fix for this ticket).

            Show
            rowen Russell Owen added a comment - Note: after issuing "rebuild starlink_ast" I found an updated "component.xml" file that incremented the version from 8.5 to 8.5.1. I committed it, but I'm wondering if should be omitted from the github repo (e.g. if it's automatically created). The version of AST is indeed 8.5.1, and some changes have been made since that version (including the fix for this ticket).
            Hide
            tjenness Tim Jenness added a comment -

            They are committed to the upstream repository because they are used by the dependency indexer for the Starlink global build. David must have forgotten to commit it upstream. It does get recreated by the build so you could remove it but it may cause annoyance each time it is updated upstream and you merge to the LSST branch.

            Show
            tjenness Tim Jenness added a comment - They are committed to the upstream repository because they are used by the dependency indexer for the Starlink global build. David must have forgotten to commit it upstream. It does get recreated by the build so you could remove it but it may cause annoyance each time it is updated upstream and you merge to the LSST branch.
            Hide
            tjenness Tim Jenness added a comment - - edited

            I'm happy to merge upstream AST. astshim changes look ok.

            Show
            tjenness Tim Jenness added a comment - - edited I'm happy to merge upstream AST. astshim changes look ok.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                Tim Jenness
                Watchers:
                John Parejko, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel