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

Epoch is not getting passed consistently among reference catalog loading functions

    XMLWordPrintable

    Details

    • Story Points:
      5
    • Sprint:
      DRP S21a (Dec Jan)
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      In looking through the code that does the reference catalog loading, it seems that the observation epoch is not getting passed around consistently among the functions that accept is as an argument. The epoch is used to correct the reference catalogs for proper motion and parallax (when present, which is the case for the Gaia reference catalogs now in more regular use with the LSST Science Pipelines).  For example, it is not getting passed to meas_algorithms's loadSkyCircle() function from within the loadPixelBox() function implementation in LoadReferenceObjectsTask here. As such, even if the latter gets passed an epoch, no corrections get applied to the reference catalog coordinates upon loading.

      Another instance is the getMetadataBox() function not passing it along to getMetadataCircle() here.

      It also seems it could be added to the loadAndMatch() function in meas_astrom's ref_mach.py here because the Struct returned by its _getExposureMetadata() function does return an epoch attribute (when available for the given exposure).

      This ticket is to clean up this situation such that the epoch gets consistently passed to and among the various functions that would make use of it.  Also add a test to check that the epoch is having an effect when passed to loadPixelBox(). It looks like this can be easily added in the testLoadPixelBox() function in test_htmIndex.py.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            Please file a ticket to write tests for the gen3 reference loader. I also noted on the PR about getting a ticket to fix our broken use of EPOCH in that metadata.

            Show
            Parejkoj John Parejko added a comment - Please file a ticket to write tests for the gen3 reference loader. I also noted on the PR about getting a ticket to fix our broken use of EPOCH in that metadata.
            Hide
            Parejkoj John Parejko added a comment -

            Thank you for doing this work. I'm not happy about the testing situation, but that's not really your fault.

            I guess this gives us a reason to accelerate implementation of DM-27013?

            Show
            Parejkoj John Parejko added a comment - Thank you for doing this work. I'm not happy about the testing situation, but that's not really your fault. I guess this gives us a reason to accelerate implementation of DM-27013 ?
            Hide
            lauren Lauren MacArthur added a comment -

            Thanks for the review, John.  As to the whole "EPOCH" and what it does/should refer to calamity, I need some help from you as I thought I was "following your lead" (where, really I mean I was taking examples from jointcal, more below) in the expectations of what we are passing to the proper motion correction implementation as a variable called "epoch".  It seems to me that many[most/all?] places that we pass around something with the variable name epoch, we do indeed mean for it to be MDJ.  To start, the Gaia refcats have entries like:

            In [52]: gaiaRefCatDf['epoch']                                                                               
            Out[52]: 
            id
            3836068354164430080    57205.874409
            3836070896784313984    57205.874409
                                       ...     
            

            which, at first glance, look like MJD to me...but looking at the documentation (warning before clicking...big file) here

            EPOCH : Gaia-centric epoch TCB(Gaia) (double, Time[Barycentric JD in TCB - 2455197.5 (day)])
            

            whatever that implies for code in applyProperMotionsImpl noted below...
            On the other hand, ps1 has:

            In [53]: ps1RefCatDf['epoch']                                                                                
            Out[53]: 
            id
            110681501036355632    1320442112
            110671501040283925    1397608704
                                     ...    
            

            which is a bit mysterious...
            Regardless, looking at applyProperMotionsImpl, the following seems to imply units of MJD for the "epoch" of both the reference and source catalogs:

            catEpoch = astropy.time.Time(catalog["epoch"], scale="tai", format="mjd")
            # Use `epoch.tai` to make sure the time difference is in TAI
            timeDiffsYears = (epoch.tai - catEpoch).to(astropy.units.yr).value 
            

            so these (catEpoch and epoch) would need to be in consistent "units". Also, in jointcal, it again seems to expect and use MDJ as what is represented by the term "epoch"

                def _compute_proper_motion_epoch(self, ccdImageList):
                    """Return the proper motion correction epoch of the provided images.
                    Parameters
                    ----------
                    ccdImageList : `list` [`lsst.jointcal.CcdImage`]
                        The images to compute the appropriate epoch for.
                    Returns
                    -------
                    epoch : `astropy.time.Time`
                        The date to use for proper motion corrections.
                    """
                    mjds = [ccdImage.getMjd() for ccdImage in ccdImageList]
                    return astropy.time.Time(np.mean(mjds), format='mjd', scale="tai")
            

            I also see your (and TimJ's) point about "standards" and expectations for such terms from the community, but we seem to be in a bit of a mess here (and I also appreciate your point that daf_base adopts "EPOCH is Julian epoch year"), and I'm quite sure I'm not the one to make any calls on changing things (I'm well-enough placed to make a best attempt at keeping all uses within our codebase consistent, but have no horse in the race as to how they should be defined and used).

            I could easily make a ticket to say we should "standardize the use of the term EPOCH, whether it is being used as a variable name for use in computations or metadata header that gets stored", but if you are advocating (quoting from the PR):

            ...to remove that epoch keyword and replace it with either EQUINOX (following the FITS standard) which would then be a Julian year, or MJD if we want to keep it as MJD. Julian year makes more sense to me as it is the value you would directly use in proper motion calculations, with values given in mas/year.

            I might ask you to file it with your desiderata and their reasonings (I don't feel comfortable suggesting the above since I'm not convinced/qualified to judge the fidelity of that claim...man, I can't even say for sure what is in the Gaia catalogs as "epoch", and that is a crucial piece of understanding required if we're going to get this right!)

            Show
            lauren Lauren MacArthur added a comment - Thanks for the review, John.  As to the whole "EPOCH" and what it does/should refer to calamity, I need some help from you as I thought I was "following your lead" (where, really I mean I was taking examples from jointcal , more below) in the expectations of what we are passing to the proper motion correction implementation as a variable called "epoch".  It seems to me that many [most/all?] places that we pass around something with the variable name epoch , we do indeed mean for it to be MDJ.  To start, the Gaia refcats have entries like: In [ 52 ]: gaiaRefCatDf[ 'epoch' ] Out[ 52 ]: id 3836068354164430080 57205.874409 3836070896784313984 57205.874409 ... which, at first glance, look like MJD to me...but looking at the documentation (warning before clicking...big file) here EPOCH : Gaia-centric epoch TCB(Gaia) (double, Time[Barycentric JD in TCB - 2455197.5 (day)]) whatever that implies for code in applyProperMotionsImpl noted below... On the other hand, ps1 has: In [ 53 ]: ps1RefCatDf[ 'epoch' ] Out[ 53 ]: id 110681501036355632 1320442112 110671501040283925 1397608704 ... which is a bit mysterious... Regardless, looking at applyProperMotionsImpl , the following seems to imply units of MJD for the "epoch" of both the reference and source catalogs: catEpoch = astropy.time.Time(catalog[ "epoch" ], scale = "tai" , format = "mjd" ) # Use `epoch.tai` to make sure the time difference is in TAI timeDiffsYears = (epoch.tai - catEpoch).to(astropy.units.yr).value so these ( catEpoch and epoch ) would need to be in consistent "units". Also, in jointcal , it again seems to expect and use MDJ as what is represented by the term "epoch" def _compute_proper_motion_epoch( self , ccdImageList): """Return the proper motion correction epoch of the provided images. Parameters ---------- ccdImageList : `list` [`lsst.jointcal.CcdImage`] The images to compute the appropriate epoch for. Returns ------- epoch : `astropy.time.Time` The date to use for proper motion corrections. """ mjds = [ccdImage.getMjd() for ccdImage in ccdImageList] return astropy.time.Time(np.mean(mjds), format = 'mjd' , scale = "tai" ) I also see your (and TimJ's) point about "standards" and expectations for such terms from the community, but we seem to be in a bit of a mess here (and I also appreciate your point that daf_base adopts "EPOCH is Julian epoch year"), and I'm quite sure I'm not the one to make any calls on changing things (I'm well-enough placed to make a best attempt at keeping all uses within our codebase consistent, but have no horse in the race as to how they should be defined and used). I could easily make a ticket to say we should "standardize the use of the term EPOCH, whether it is being used as a variable name for use in computations or metadata header that gets stored", but if you are advocating (quoting from the PR): ...to remove that epoch keyword and replace it with either EQUINOX (following the FITS standard) which would then be a Julian year, or MJD if we want to keep it as MJD. Julian year makes more sense to me as it is the value you would directly use in proper motion calculations, with values given in mas/year. I might ask you to file it with your desiderata and their reasonings (I don't feel comfortable suggesting the above since I'm not convinced/qualified to judge the fidelity of that claim...man, I can't even say for sure what is in the Gaia catalogs as "epoch", and that is a crucial piece of understanding required if we're going to get this right!)
            Hide
            lauren Lauren MacArthur added a comment -

            Ticket for Gen3 reference loader tests is DM-28321.

            Show
            lauren Lauren MacArthur added a comment - Ticket for Gen3 reference loader tests is DM-28321 .
            Hide
            Parejkoj John Parejko added a comment -

            Ok, I did the deep dive on uses of "epoch" in the stack. Looks like the only real culprits are the one we found here, and some in sdm_schemas/dax_apdb. See DM-28329 for (many) details.

            Show
            Parejkoj John Parejko added a comment - Ok, I did the deep dive on uses of "epoch" in the stack. Looks like the only real culprits are the one we found here, and some in sdm_schemas/dax_apdb. See DM-28329 for (many) details.

              People

              Assignee:
              lauren Lauren MacArthur
              Reporter:
              lauren Lauren MacArthur
              Reviewers:
              John Parejko
              Watchers:
              Chris Morrison, Jim Bosch, John Parejko, Lauren MacArthur
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.