# ChebyshevBoundedField should use _ not . as field separators for persistence

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
1
• Sprint:
Science Pipelines DM-S15-5
• Team:

#### 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.

#### Activity

No builds found.
Russell Owen created issue -
Field Original Value New Value
Link This issue blocks DM-2981 [ DM-2981 ]
 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. 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.
 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. 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.
Hide
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.

Show
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.
Hide
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 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 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 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) 

Show
Hide
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.

Show
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.
 Assignee Russell Owen [ rowen ]
 Status To Do [ 10001 ] In Progress [ 3 ]
 Sprint Science Pipelines DM-S15-5 [ 162 ] Story Points 1 Team Alert Production [ 10300 ]
Hide
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.

Show
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 .
 Resolution Done [ 10000 ] Status In Progress [ 3 ] Done [ 10002 ]
 Epic Link DM-1991 [ 16080 ]
 Link This issue is triggered by RFC-58 [ RFC-58 ]

#### People

Assignee:
Russell Owen
Reporter:
Russell Owen
Watchers:
Jim Bosch, John Swinbank, Russell Owen, Simon Krughoff, Yusra AlSayyad