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

Stop using boost persistence in afw

    XMLWordPrintable

    Details

      Description

      Instead of using ExposureFormatter and the like it would be simpler for the butler to run FITS persistence directly. My proposal is as follows:

      • In afw add Python functions readFitsWithOptions(path, options) and writeFitsWithOptions(path, options) to all classes that the butler may persist, including Exposure, MaskedImage, Image, Mask and all catalog classes,
      • In those new functions the options argument is a daf.base.PropertySet containing the "additional data" provided to the butler. The functions parse that data and use that to call readFits or writeFits method appropriately.
      • Note that the path argument can be anything supported by an overload of readFits or writeFits: a file path, fits::File or fits::MemFileManager.
      • In daf_persistence the Fits IO functions will be simplified to call the new functions described above, and the same pair of functions can be used for image-like objects and catalogs.

      A similar treatment will be required for other objects that the butler tries to persist as FITS using boost persistence.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Just linked in DM-14504. Not sure how much that ticket overlaps with this one - it's more about code removal - but if you end up doing all of it, please close it appropriately.

            Show
            jbosch Jim Bosch added a comment - Just linked in DM-14504 . Not sure how much that ticket overlaps with this one - it's more about code removal - but if you end up doing all of it, please close it appropriately.
            Hide
            rowen Russell Owen added a comment -

            Jim Bosch since this work is transitional, I'm wondering how far to take it. It is clearly required for the image-like classes in order to get rid of their boost formatters, since the existing Gen2 butler code uses those formatters. But is there a reason to change I/O for catalogs? The existing Gen2 butler code does not use formatters for that. If it would help Gen3 butler in some way then fine, but otherwise I prefer to make the minimal changes required to get rid of boost formatters in afw.

            Show
            rowen Russell Owen added a comment - Jim Bosch since this work is transitional, I'm wondering how far to take it. It is clearly required for the image-like classes in order to get rid of their boost formatters, since the existing Gen2 butler code uses those formatters. But is there a reason to change I/O for catalogs? The existing Gen2 butler code does not use formatters for that. If it would help Gen3 butler in some way then fine, but otherwise I prefer to make the minimal changes required to get rid of boost formatters in afw.
            Hide
            jbosch Jim Bosch added a comment -

            I don't think there's any reason to change I/O for catalogs now.

            Show
            jbosch Jim Bosch added a comment - I don't think there's any reason to change I/O for catalogs now.
            Hide
            rowen Russell Owen added a comment - - edited

            Most of the work was done on afw on this pull request: https://github.com/lsst/afw/pull/389
            A small change in utils was required, though I admit I don't know why

            Show
            rowen Russell Owen added a comment - - edited Most of the work was done on afw on this pull request: https://github.com/lsst/afw/pull/389 A small change in utils was required, though I admit I don't know why
            Hide
            rowen Russell Owen added a comment - - edited

            A few comments:

            • I also removed boost persistence support from meas_algorithms (e.g. PsfFormatter)
            • The reader in fitsIoWithOptions originally handled the "hdu" option as well, but it turns out "hdu" is handled somewhere else (as I should have noticed when reading the old formatters). That seems weird to me but I left it.

            At this point we may be able to remove boost persistence support from daf_persistence and daf_base (e.g. PropertySet's handling of pointers to Persistable objects). But that would be a different ticket. I'm not entirely sure it's safe to do, either – pex_policy makes some use of pointer-to-persistable and I have no idea if that code is actually being used.

            Show
            rowen Russell Owen added a comment - - edited A few comments: I also removed boost persistence support from meas_algorithms (e.g. PsfFormatter) The reader in fitsIoWithOptions originally handled the "hdu" option as well, but it turns out "hdu" is handled somewhere else (as I should have noticed when reading the old formatters). That seems weird to me but I left it. At this point we may be able to remove boost persistence support from daf_persistence and daf_base (e.g. PropertySet's handling of pointers to Persistable objects). But that would be a different ticket. I'm not entirely sure it's safe to do, either – pex_policy makes some use of pointer-to-persistable and I have no idea if that code is actually being used.
            Hide
            jbosch Jim Bosch added a comment -

            Looks good overall; only minor comments in afw.  I'd like to understand what necessitated the changes in utils and see if we can come up with something a bit less empirical.  The old functions in daf_persistence may just need different names and descriptions, but something is awry there.

            Show
            jbosch Jim Bosch added a comment - Looks good overall; only minor comments in afw.  I'd like to understand what necessitated the changes in utils and see if we can come up with something a bit less empirical.  The old functions in daf_persistence may just need different names and descriptions, but something is awry there.
            Hide
            rowen Russell Owen added a comment -

            Thank you for a very helpful review.

            Show
            rowen Russell Owen added a comment - Thank you for a very helpful review.

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Kian-Tat Lim, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: