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

ChebyshevBoundedField should use _ not . as field separators for persistence

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • afw
    • None

    Description

      ChebyshevBoundedField uses "." instead of "_" as field separators in its afw table persistence. This is the old way of doing things, and unfortunately causes errors when reading in older versions of tables, becaus afw converts "." to "_" in that situation.

      This shows up as a unit test failure in DM-2981 (brought over from HSC) when an older version table is read in.

      It is an open question whether to fix this as part of DM-2981 (which conveniently has a test that shows the problem, though not intentionally so) or separately, in which case a new test is wanted. In the former case I'm happy to do the work so I can finish DM-2981.

      Many thanks to Jim Bosch for diagnosing the problem.

      Attachments

        Issue Links

          Activity

            Forgive my naivety, but on first reading I'm not sure that I follow.

            You mention that "older version table is read in". Do you mean that this test relies on some old table that has been saved in afwdata or equivalent, and that breaks? Or do you just mean that when you persist a new ChebyshevBoundedField it makes an old-style table that you can't then successfully read back?

            In the latter case, I guess this is pretty straightforward fix (I guess it just needs a few . s to be replaced by _ s in ChebyshevBoundedField.cc). I'd suggest filing a new ticket for it, but would imagine it could be turned around quickly.

            If there's some deeper backwards compatibility issue here, that might be a different matter.

            swinbank John Swinbank added a comment - Forgive my naivety, but on first reading I'm not sure that I follow. You mention that "older version table is read in". Do you mean that this test relies on some old table that has been saved in afwdata or equivalent, and that breaks? Or do you just mean that when you persist a new ChebyshevBoundedField it makes an old-style table that you can't then successfully read back? In the latter case, I guess this is pretty straightforward fix (I guess it just needs a few . s to be replaced by _ s in ChebyshevBoundedField.cc ). I'd suggest filing a new ticket for it, but would imagine it could be turned around quickly. If there's some deeper backwards compatibility issue here, that might be a different matter.
            rowen Russell Owen added a comment - - edited

            An existing unit test from HSC triggers this issue (see below). The test intentionally tries to unpersist an ExposureInfo table that uses an older table format. The test fails while trying to unpersist a ChebyshevBoundedField, which is presumably part of an ApCorrMap.

            Jim Bosch understands the details better than I do, but I think what is happening is that when afw table reads an older format of data it auto-converts field separators "." into "_". That fails for ChebyshevBoundedField because it incorrectly uses "." instead of "_".

            In any case it is a certainty that ChebyshevBoundedField should use "_" instead of "." as its field separator, and the current code is incorrect (probably because that was not changed when it was brought over from HSC, which uses the older field separator).

            tests/testValidPolygon.py
             
            E: Empty WCS extension, using FITS header
            ....
            ======================================================================
            ERROR: testExposureCatalogBackwardsCompatibility (__main__.ValidPolygonTestCase)
            Test that we can read an ExposureCatalog written with an old version of the code.
            ----------------------------------------------------------------------
            Traceback (most recent call last):
             File "tests/testValidPolygon.py", line 96, in testExposureCatalogBackwardsCompatibility
               cat2 = afwTable.ExposureCatalog.readFits(filename2)
             File "/Users/rowen/LSST/lsstsw/build/afw/python/lsst/afw/table/tableLib.py", line 11090, in readFits
               return _tableLib.ExposureCatalog_readFits(*args)
            NotFoundError: 
             File "src/table/Schema.cc", line 239, in SchemaItem<T> lsst::afw::table::detail::SchemaImpl::find(const std::string &) const [T = int]
               Field or subfield withname 'order.x' not found with type 'I'. {0}
             File "src/table/io/InputArchive.cc", line 105, in boost::shared_ptr<Persistable> lsst::afw::table::io::InputArchive::Impl::get(int, const lsst::afw::table::io::InputArchive &)
               loading object with id=2, name='ChebyshevBoundedField' {1}
             File "src/table/io/InputArchive.cc", line 105, in boost::shared_ptr<Persistable> lsst::afw::table::io::InputArchive::Impl::get(int, const lsst::afw::table::io::InputArchive &)
               loading object with id=1, name='ApCorrMap' {2}
            lsst::pex::exceptions::NotFoundError: 'Field or subfield withname 'order.x' not found with type 'I'. {0}; loading object with id=2, name='ChebyshevBoundedField' {1}; loading object with id=1, name='ApCorrMap' {2}'
             
             
            ----------------------------------------------------------------------
            Ran 5 tests in 0.049s
             
            FAILED (errors=1)
            

            rowen Russell Owen added a comment - - edited An existing unit test from HSC triggers this issue (see below). The test intentionally tries to unpersist an ExposureInfo table that uses an older table format. The test fails while trying to unpersist a ChebyshevBoundedField, which is presumably part of an ApCorrMap. Jim Bosch understands the details better than I do, but I think what is happening is that when afw table reads an older format of data it auto-converts field separators "." into "_". That fails for ChebyshevBoundedField because it incorrectly uses "." instead of "_". In any case it is a certainty that ChebyshevBoundedField should use "_" instead of "." as its field separator, and the current code is incorrect (probably because that was not changed when it was brought over from HSC, which uses the older field separator). tests/testValidPolygon.py   E: Empty WCS extension, using FITS header .... ====================================================================== ERROR: testExposureCatalogBackwardsCompatibility (__main__.ValidPolygonTestCase) Test that we can read an ExposureCatalog written with an old version of the code. ---------------------------------------------------------------------- Traceback (most recent call last): File "tests/testValidPolygon.py", line 96, in testExposureCatalogBackwardsCompatibility cat2 = afwTable.ExposureCatalog.readFits(filename2) File "/Users/rowen/LSST/lsstsw/build/afw/python/lsst/afw/table/tableLib.py", line 11090, in readFits return _tableLib.ExposureCatalog_readFits(*args) NotFoundError: File "src/table/Schema.cc", line 239, in SchemaItem<T> lsst::afw::table::detail::SchemaImpl::find(const std::string &) const [T = int] Field or subfield withname 'order.x' not found with type 'I'. {0} File "src/table/io/InputArchive.cc", line 105, in boost::shared_ptr<Persistable> lsst::afw::table::io::InputArchive::Impl::get(int, const lsst::afw::table::io::InputArchive &) loading object with id=2, name='ChebyshevBoundedField' {1} File "src/table/io/InputArchive.cc", line 105, in boost::shared_ptr<Persistable> lsst::afw::table::io::InputArchive::Impl::get(int, const lsst::afw::table::io::InputArchive &) loading object with id=1, name='ApCorrMap' {2} lsst::pex::exceptions::NotFoundError: 'Field or subfield withname 'order.x' not found with type 'I'. {0}; loading object with id=2, name='ChebyshevBoundedField' {1}; loading object with id=1, name='ApCorrMap' {2}'     ---------------------------------------------------------------------- Ran 5 tests in 0.049s   FAILED (errors=1)

            After talking to Simon I will take this on. If the fix is as trivial as I think it will be then I will included it on tickets/DM-2981. If it proves less tractable then i will fix it on a separate ticket branch.

            rowen Russell Owen added a comment - After talking to Simon I will take this on. If the fix is as trivial as I think it will be then I will included it on tickets/ DM-2981 . If it proves less tractable then i will fix it on a separate ticket branch.
            rowen Russell Owen added a comment -

            Fixed as a trivial change to ChebyshevBoundedField.cc in afw on tickets/DM-2981. A unit test on that branch failed before the fix (due to this problem) and worked afterwards. Paul Price reviewed the change as part of reviewing DM-2981.

            rowen Russell Owen added a comment - Fixed as a trivial change to ChebyshevBoundedField.cc in afw on tickets/ DM-2981 . A unit test on that branch failed before the fix (due to this problem) and worked afterwards. Paul Price reviewed the change as part of reviewing DM-2981 .

            People

              rowen Russell Owen
              rowen Russell Owen
              Jim Bosch, John Swinbank, Russell Owen, Simon Krughoff (Inactive), Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.