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

FIx invalid unit string of "second" in meas catalogs

    Details

    • Team:
      Data Release Production

      Description

      Per this review comment, in HSC reprocessing results, columns 'modelfit_CModel_dev_time', 'model fit_CModel_initial_time', and model fit_CModel_expTime' cause exceptions to be raised when converting a SourceCatalog to an astropy.table.Table and then to an astropy.vo.VOTableFile. The exception raised is that the unit string of "second" is not valid.

      The specific data item where this was found can be retrieved with:

      from lsst.daf.persistence import Butler
      butler = Butler('/datasets/hsc/repo/rerun/RC/w_2018_38/DM-15690')
      dataId = dict(filter='HSC-R',  tract=9813, patch='4,4')
      measCat = butler.get(deepCoadd_meas', dataId=dataId)
      

        Attachments

          Issue Links

            Activity

            Hide
            shupe David Shupe added a comment -

            For recent stack versions (e.g. weekly 2018_44 and later) I am not getting any warnings about this anymore. John Swinbank what is the right way to close this ticket?

            Show
            shupe David Shupe added a comment - For recent stack versions (e.g. weekly 2018_44 and later) I am not getting any warnings about this anymore. John Swinbank what is the right way to close this ticket?
            Hide
            swinbank John Swinbank added a comment -

            If the ticket has been fixed elsewhere, close it as a duplicate:

            If you discover that a ticket duplicates another one, you can retire the duplicate ticket by marking it as Invalid. Name the duplicate ticket in the status change comment field.

            https://developer.lsst.io/work/flow.html#ticket-status

            But... has this really been fixed? To my inexpert eye it looks as though meas_modelfit is still writing units as second, and Astropy is still unhappy with that:

            In [9]: from_table(t)
            WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree]
            Out[9]: <VOTABLE>... 1 tables ...</VOTABLE>
            

            Did I miss something?

            Show
            swinbank John Swinbank added a comment - If the ticket has been fixed elsewhere, close it as a duplicate: If you discover that a ticket duplicates another one, you can retire the duplicate ticket by marking it as Invalid. Name the duplicate ticket in the status change comment field. https://developer.lsst.io/work/flow.html#ticket-status But... has this really been fixed? To my inexpert eye it looks as though meas_modelfit is still writing units as second , and Astropy is still unhappy with that: In [ 9 ]: from_table(t) WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree] Out[ 9 ]: <VOTABLE>... 1 tables ...< / VOTABLE> Did I miss something?
            Hide
            shupe David Shupe added a comment -

            git blame says Pim Schellart [X] changed the unit from 'seconds' to 'second' in 2016. I think Astropy doesn't complain about 'second'.

            [34]: import astropy.units as u
            [35]: u.s.long_names
            ['second']
            [36]: u.s.names
            ['s', 'second']
            

            Show
            shupe David Shupe added a comment - git blame says Pim Schellart [X] changed the unit from 'seconds' to 'second' in 2016. I think Astropy doesn't complain about 'second' . [34]: import astropy.units as u [35]: u.s.long_names ['second'] [36]: u.s.names ['s', 'second']
            Hide
            swinbank John Swinbank added a comment -

            But:

            $ eups list -s lsst_distrib
               16.0-2-g3466de9+2    current w_2018_47 setup
             
            $ cat test_second.py 
            from lsst.daf.persistence import Butler
            from astropy.table import Table
            from astropy.io.votable import from_table
             
            butler = Butler('/datasets/hsc/repo/rerun/RC/w_2018_38/DM-15690')
            dataId = dict(filter='HSC-R',  tract=9813, patch='4,4')
            measCat = butler.get('deepCoadd_meas', dataId=dataId)
            measCat.schema['modelfit_CModel_initial_time'].asField()
            measCat.writeFits("measCat.fits")
            t = Table.read("measCat.fits", hdu=1)
            from_table(t)
             
            $ python test_second.py 
            CameraMapper INFO: Loading exposure registry from /datasets/hsc/repo/registry.sqlite3
            CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3
            CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3
            WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree]
            WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree]
            

            Show
            swinbank John Swinbank added a comment - But: $ eups list - s lsst_distrib 16.0 - 2 - g3466de9 + 2 current w_2018_47 setup   $ cat test_second.py from lsst.daf.persistence import Butler from astropy.table import Table from astropy.io.votable import from_table   butler = Butler( '/datasets/hsc/repo/rerun/RC/w_2018_38/DM-15690' ) dataId = dict ( filter = 'HSC-R' , tract = 9813 , patch = '4,4' ) measCat = butler.get( 'deepCoadd_meas' , dataId = dataId) measCat.schema[ 'modelfit_CModel_initial_time' ].asField() measCat.writeFits( "measCat.fits" ) t = Table.read( "measCat.fits" , hdu = 1 ) from_table(t)   $ python test_second.py CameraMapper INFO: Loading exposure registry from / datasets / hsc / repo / registry.sqlite3 CameraMapper INFO: Loading calib registry from / datasets / hsc / repo / CALIB / calibRegistry.sqlite3 CameraMapper INFO: Loading calib registry from / datasets / hsc / repo / CALIB / calibRegistry.sqlite3 WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree] WARNING: W50: ?:?:?: W50: Invalid unit string 'second' [astropy.io.votable.tree]
            Hide
            shupe David Shupe added a comment -

            That explains why the warning went away for me. I switched from writing measCat to disk and reading it in as an Astropy table, to using measCat.asAstropy(). I have to change dtypes that are int64 to make the conversion to VOTable work.

            $ eups list -s lsst_distrib
               16.0-2-g3466de9+2    current w_2018_47 setup
             
            $ cat test_second_mem.py 
            from lsst.daf.persistence import Butler
            from astropy.table import Table
            from astropy.io.votable import from_table
            import numpy as np
             
            butler = Butler('/datasets/hsc/repo/rerun/RC/w_2018_38/DM-15690')
            dataId = dict(filter='HSC-R',  tract=9813, patch='4,4')
            measCat = butler.get('deepCoadd_meas', dataId=dataId)
            measCat.schema['modelfit_CModel_initial_time'].asField()
            t = measCat.asAstropy()
            for c in t.colnames:
                if t[c].dtype.num == 9:
                    t[c].dtype = np.dtype('long')
            from_table(t)
             
            $ python test_second_mem.py 
            CameraMapper INFO: Loading exposure registry from /datasets/hsc/repo/registry.sqlite3
            CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3
            CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3
            

            Show
            shupe David Shupe added a comment - That explains why the warning went away for me. I switched from writing measCat to disk and reading it in as an Astropy table, to using measCat.asAstropy() . I have to change dtypes that are int64 to make the conversion to VOTable work. $ eups list -s lsst_distrib 16.0-2-g3466de9+2 current w_2018_47 setup $ cat test_second_mem.py from lsst.daf.persistence import Butler from astropy.table import Table from astropy.io.votable import from_table import numpy as np butler = Butler('/datasets/hsc/repo/rerun/RC/w_2018_38/DM-15690') dataId = dict(filter='HSC-R', tract=9813, patch='4,4') measCat = butler.get('deepCoadd_meas', dataId=dataId) measCat.schema['modelfit_CModel_initial_time'].asField() t = measCat.asAstropy() for c in t.colnames: if t[c].dtype.num == 9: t[c].dtype = np.dtype('long') from_table(t)   $ python test_second_mem.py CameraMapper INFO: Loading exposure registry from /datasets/hsc/repo/registry.sqlite3 CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3 CameraMapper INFO: Loading calib registry from /datasets/hsc/repo/CALIB/calibRegistry.sqlite3
            Hide
            tjenness Tim Jenness added a comment -

            I think the unit string should be "s" and not "second". Are we still writing this incorrectly somewhere?

            Show
            tjenness Tim Jenness added a comment - I think the unit string should be "s" and not "second". Are we still writing this incorrectly somewhere?
            Show
            swinbank John Swinbank added a comment - Look like at https://github.com/lsst/meas_modelfit/blob/master/src/CModel.cc#L207 , maybe?

              People

              • Assignee:
                Unassigned
                Reporter:
                shupe David Shupe
                Watchers:
                David Shupe, Jim Bosch, John Swinbank, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated: