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

Problems with EFD timestamps and/or code

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • efd
    • None
    • SQuaRE
    • No

    Description

      I've been looking at the EFD timebase issues, and I believe there are definitely issues that need fixing. Perhaps I am confused, if so I would appreciate it if someone could point me at a description of how this is all supposed to work. I looked at an image from the latest AuxTel observing run on 04-Aug-21. I tried to distill the notebook down to the minimum necessary to see the problems. It is at this location (https://github.com/craiglagegit/ScratchStuff/blob/master/notebooks/Plot_Tracking_3_13Aug21.ipynb), or available at NCSA at /project/cslage/AuxTel/notebooks/Plot_Tracking_3_13Aug21.ipynb . I see three problems:

      (1) Why is the DATE keyword in the header ~30 seconds before DATE-BEG and DATE-END ?
      (2) I need to lie to EfdClient.select_time_series and tell it the UTC timestamps are in TAI, otherwise I get a warning and the whole dataset is shifted by ~30 seconds.
      (3) I need to override the default for internal_time_scale in merge_packed_time_series. The default is TAI, but I need to make it UTC for it all to work.

      If I do all of those things then the plots make sense. The plot below shows an exposure where after a small AzEl adjustment there is an allAxesInPosition timestamp followed by an exposure. The DATE-BEG and DATE-END timestamps in the header line up within milliseconds with the lsst.sal.ATCamera.logevent_shutterDetailedState timestamps in the EFD.

      Attachments

        Issue Links

          Activity

            tjenness Tim Jenness added a comment -

            Fixing my incorrect comment that I have now deleted.

            DATE is the date the file is written and should clearly occur after DATE-END. For consistency with the standard I believe it should be in the same time system as indicated by TIMESYS header and so should be in TAI.

            tjenness Tim Jenness added a comment - Fixing my incorrect comment that I have now deleted. DATE is the date the file is written and should clearly occur after DATE-END. For consistency with the standard I believe it should be in the same time system as indicated by TIMESYS header and so should be in TAI.
            tjenness Tim Jenness added a comment -

            I'm not entirely sure which team is meant to be looking at this. I think the DATE keyword comes from whoever is writing the FITS files. spietrowicz is that added by the Archiver or is that added directly by CFITSIO? If it's CFITSIO I may need to take another look at the FITS standard and how DATE is affected by the TIMESYS header.

            I'm also not sure if the EFD questions are something for afausti to answer or something on the CSC end being incorrect.

            tjenness Tim Jenness added a comment - I'm not entirely sure which team is meant to be looking at this. I think the DATE keyword comes from whoever is writing the FITS files. spietrowicz is that added by the Archiver or is that added directly by CFITSIO? If it's CFITSIO I may need to take another look at the FITS standard and how DATE is affected by the TIMESYS header. I'm also not sure if the EFD questions are something for afausti to answer or something on the CSC end being incorrect.
            spietrowicz Steve Pietrowicz added a comment - - edited

            The DATE field comes from the HeaderService.  All header information is taken by the Forwarder and written for the files, and isn't touched by the Forwarder.   I didn't write the Forwarder, but looked through the code, and didn't see any special treatment for DATE.

            spietrowicz Steve Pietrowicz added a comment - - edited The DATE field comes from the HeaderService.  All header information is taken by the Forwarder and written for the files, and isn't touched by the Forwarder.   I didn't write the Forwarder, but looked through the code, and didn't see any special treatment for DATE.
            tjenness Tim Jenness added a comment -

            I'm pretty sure the file writing should not be using the DATE field from the header service. The standard says it's meant to be the date the file is written, not the date the header is constructed.

            tjenness Tim Jenness added a comment - I'm pretty sure the file writing should not be using the DATE field from the header service. The standard says it's meant to be the date the file is written, not the date the header is constructed.
            pingraham Patrick Ingraham added a comment - - edited

            The `select_packed_time_series` in the EFDClient uses the cRIO_timestamp which is known to have a 37s offset. This is an ATMCS bug that is in the queue to get fixed.

            This was deemed to be incorrect and was due to a bug in astropy version 4.1 in the handling of TAI. 

            pingraham Patrick Ingraham added a comment - - edited The `select_packed_time_series` in the EFDClient uses the cRIO_timestamp which is known to have a 37s offset. This is an ATMCS bug that is in the queue to get fixed. This was deemed to be incorrect and was due to a bug in astropy version 4.1 in the handling of TAI. 
            tjenness Tim Jenness added a comment -

            pingraham can you link this ticket to the ATMCS ticket?

            ktl philosophically, should the DATE header be the date we wrote the file or the date the header was created. I think the FITS standard would prefer when the data file is written. If header service is forming the DATE header than that already means it will have a different value to the CCS-written file.

            tjenness Tim Jenness added a comment - pingraham can you link this ticket to the ATMCS ticket? ktl philosophically, should the DATE header be the date we wrote the file or the date the header was created. I think the FITS standard would prefer when the data file is written. If header service is forming the DATE header than that already means it will have a different value to the CCS-written file.
            spietrowicz Steve Pietrowicz added a comment - - edited

            tjenness -   If the clocks are synced, that DATE is not even the time the header file was written - if it was, the DATE would already be a later time than the others.  

             

            The question is, do we want the date at which the pixels were written to the store, before the Forwarder assembles the FITS file, or does the (not long for this world) Forwarder have to be altered to create a DATE when it writes the file.  As you pointed out about the header service, if the Forwarder writes the DATE, it's going to be different than the CCS-written file.

            Or do we leave the Forwarder as is, since come DMTN-147, it's going away completely.

            spietrowicz Steve Pietrowicz added a comment - - edited tjenness  -   If the clocks are synced, that DATE is not even the time the header file was written - if it was, the DATE would already be a later time than the others.     The question is, do we want the date at which the pixels were written to the store, before the Forwarder assembles the FITS file, or does the (not long for this world) Forwarder have to be altered to create a DATE when it writes the file.  As you pointed out about the header service, if the Forwarder writes the DATE, it's going to be different than the CCS-written file. Or do we leave the Forwarder as is, since come DMTN-147, it's going away completely.
            tjenness Tim Jenness added a comment -

            I read the FITS standard and it says:

            The value of the DATE keyword shall always be expressed in UTC when in this format, for all data sets created on Earth.

            So it explicitly ignores TIMESYS header and is correctly being written as UTC here. I think that means everything is right from the point of view of this ticket. The philosophical question can likely be punted (and I'm fine with forward leaving it alone).

            The standard also says:

            The value field shall contain a character string giving the date on which the HDU was created...
            When a newly created HDU is substantially a verbatim copy of another HDU, the value of the DATE keyword in the original HDU may be retained in the new HDU instead of updating the value to the current date and time.

            So that leaves things open to interpretation. You could take it to mean the date the pixels were written to the store or the date the HDU was formed to be written to the file.

            tjenness Tim Jenness added a comment - I read the FITS standard and it says: The value of the DATE keyword shall always be expressed in UTC when in this format, for all data sets created on Earth. So it explicitly ignores TIMESYS header and is correctly being written as UTC here. I think that means everything is right from the point of view of this ticket. The philosophical question can likely be punted (and I'm fine with forward leaving it alone). The standard also says: The value field shall contain a character string giving the date on which the HDU was created... When a newly created HDU is substantially a verbatim copy of another HDU, the value of the DATE keyword in the original HDU may be retained in the new HDU instead of updating the value to the current date and time. So that leaves things open to interpretation. You could take it to mean the date the pixels were written to the store or the date the HDU was formed to be written to the file.
            afausti Angelo Fausti added a comment - - edited

            cslage in cell 5:

            # The result says that this timestamp is in UTC.
            inPosition.index[0]
            Out[5]:
            Timestamp('2021-08-05 02:07:45.404000+0000', tz='UTC')
            

            It is confusing because pandas.Timestamp does not support TAI, and assumes UTC.

            But the default behaviour (at least for the moment, see DM-30083) is that the InfluxDB time column is in TAI and so the time index in the data frame returned by the EFD client is also TAI.

            I think that answer item (2) above.

            In cell 4, you can use

            inPosition = await client.select_time_series("lsst.sal.ATMCS.logevent_allAxesInPosition", start, end) 
            

            with start and end timestamps in TAI. Let me know if that helps.

            afausti Angelo Fausti added a comment - - edited cslage in cell 5: # The result says that this timestamp is in UTC. inPosition.index[0] Out[5]: Timestamp('2021-08-05 02:07:45.404000+0000', tz='UTC') It is confusing because pandas.Timestamp does not support TAI, and assumes UTC. But the default behaviour (at least for the moment, see DM-30083 ) is that the InfluxDB time column is in TAI and so the time index in the data frame returned by the EFD client is also TAI. I think that answer item (2) above. In cell 4, you can use inPosition = await client.select_time_series("lsst.sal.ATMCS.logevent_allAxesInPosition", start, end) with start and end timestamps in TAI. Let me know if that helps.

            Adding felipe re: the DATE issue above.

            spietrowicz Steve Pietrowicz added a comment - Adding felipe  re: the DATE issue above.

            DATE is defined as "File creation date (TAI)" of the header file, see:

            https://confluence.lsstcorp.org/display/SYSENG/ComCam+Header+Information

            felipe Felipe Menanteau added a comment - DATE is defined as "File creation date (TAI)" of the header file, see: https://confluence.lsstcorp.org/display/SYSENG/ComCam+Header+Information
            tjenness Tim Jenness added a comment -

            Yes, but when the file itself is written with the pixel data DATE should be the date the HDU is written it seems. The standard implies DATE could be different for each HDU – you could make the case that the primary header should have the DATE as given by the header service but the amplifier HDUs should have the DATE for when they were created.

            tjenness Tim Jenness added a comment - Yes, but when the file itself is written with the pixel data DATE should be the date the HDU is written it seems. The standard implies DATE could be different for each HDU – you could make the case that the primary header should have the DATE as given by the header service but the amplifier HDUs should have the DATE for when they were created.
            cslage Craig Lage added a comment -

            So here's my understanding on my three original questions. Please comment if I am mistaken:
            (1) Why is the DATE keyword in the header ~30 seconds before DATE-BEG and DATE-END ?

            The reason is that DATE is in UTC and DATE-BEG and DATE-END are in TAI. The TIMESYS keyword says they all should be in TAI, but there is another spec that says DATE should be in UTC. Please tell me we don't intend to leave it this way. This will cause endless confusion going forward.

            (2) I need to lie to EfdClient.select_time_series and tell it the UTC timestamps are in TAI, otherwise I get a warning and the whole dataset is shifted by ~30 seconds.

            The EFD time stamps are actually in TAI, so I am not lying to the code when I tell them they are in TAI. The message that says they are in UTC is incorrect and comes from Pandas, so there may be no way to fix this and we will just have to live with it. Is this correct?

            (3) I need to override the default for internal_time_scale in merge_packed_time_series. The default is TAI, but I need to make it UTC for it all to work.

            I'm still not sure that I understand the resolution of this. Patrick had made an earlier comment that the cRIO ticket would fix this, but that has now been retracted. Any more understanding of this issue and how to fix it?

            cslage Craig Lage added a comment - So here's my understanding on my three original questions. Please comment if I am mistaken: (1) Why is the DATE keyword in the header ~30 seconds before DATE-BEG and DATE-END ? The reason is that DATE is in UTC and DATE-BEG and DATE-END are in TAI. The TIMESYS keyword says they all should be in TAI, but there is another spec that says DATE should be in UTC. Please tell me we don't intend to leave it this way. This will cause endless confusion going forward. (2) I need to lie to EfdClient.select_time_series and tell it the UTC timestamps are in TAI, otherwise I get a warning and the whole dataset is shifted by ~30 seconds. The EFD time stamps are actually in TAI, so I am not lying to the code when I tell them they are in TAI. The message that says they are in UTC is incorrect and comes from Pandas, so there may be no way to fix this and we will just have to live with it. Is this correct? (3) I need to override the default for internal_time_scale in merge_packed_time_series. The default is TAI, but I need to make it UTC for it all to work. I'm still not sure that I understand the resolution of this. Patrick had made an earlier comment that the cRIO ticket would fix this, but that has now been retracted. Any more understanding of this issue and how to fix it?

            I put in the comment above that there is a bug in astropy that is complicating things. The cRIO timestamps should be ok.

            See discussion here:
            https://lsstc.slack.com/archives/CM65SGAJE/p1629761457065500

             

            pingraham Patrick Ingraham added a comment - I put in the comment above that there is a bug in astropy that is complicating things. The cRIO timestamps should be ok. See discussion here: https://lsstc.slack.com/archives/CM65SGAJE/p1629761457065500  
            tjenness Tim Jenness added a comment -

            Regarding DATE the standard is very clear. The DATE header is always UTC (date of HDU creation). The "DATE-*" headers relating to acquisition follow TIMESYS.

            tjenness Tim Jenness added a comment - Regarding DATE the standard is very clear. The DATE header is always UTC (date of HDU creation). The "DATE-*" headers relating to acquisition follow TIMESYS.
            cslage Craig Lage added a comment -

            OK. Well, I guess we'll just leave it that way then. Maybe everyone else knows this and there won't be as much confusion as I think.

            cslage Craig Lage added a comment - OK. Well, I guess we'll just leave it that way then. Maybe everyone else knows this and there won't be as much confusion as I think.

            cslage yes, regarding (2) that's correct.

            However, we will switch the EFD time to UTC in the near future according to RFC-767. After that Pandas will correctly report time in UTC. Hopefully, that will improve things.

            afausti Angelo Fausti added a comment - cslage yes, regarding (2) that's correct. However, we will switch the EFD time to UTC in the near future according to RFC-767 . After that Pandas will correctly report time in UTC. Hopefully, that will improve things.
            tjenness Tim Jenness added a comment -

            What do we want to do with this ticket? (1) is working as designed. I'm not sure if (2) and (3) are covered by other tickets.

            tjenness Tim Jenness added a comment - What do we want to do with this ticket? (1) is working as designed. I'm not sure if (2) and (3) are covered by other tickets.

            (2) is covered by DM-30083. We'll be able to test a new release of the EFD client at the NCSA test stand soon (see DM-31494) and we plan on switching the InfluxDB time from TAI to UTC on the next Summit schedule maintenance which is scheduled for Oct 13.

            afausti Angelo Fausti added a comment - (2) is covered by DM-30083 . We'll be able to test a new release of the EFD client at the NCSA test stand soon (see DM-31494 ) and we plan on switching the InfluxDB time from TAI to UTC on the next Summit schedule maintenance which is scheduled for Oct 13.

            I believe (3) is also resolved by the new EFD client (DM-31494) since it now uses UTC as internal scale but krughoff can confirm that.

            afausti Angelo Fausti added a comment - I believe (3) is also resolved by the new EFD client ( DM-31494 ) since it now uses UTC as internal scale but krughoff can confirm that.
            cslage Craig Lage added a comment -

            I suggest I re-run the tests on (2) and (3) after Oct 13 or when the new EFD client is implemented. As for (1), will DATE-BEG and DATE-END change to UTC when DM-31494 is implemented, or do we plan for them to remain in TAI? If they change to UTC, we will need to change the TIMESYS keyword to be UTC.

            cslage Craig Lage added a comment - I suggest I re-run the tests on (2) and (3) after Oct 13 or when the new EFD client is implemented. As for (1), will DATE-BEG and DATE-END change to UTC when DM-31494 is implemented, or do we plan for them to remain in TAI? If they change to UTC, we will need to change the TIMESYS keyword to be UTC.
            tjenness Tim Jenness added a comment -

            (1) is working as designed. All DATE-* headers use TIMESYS which is set to TAI. The DATE header is always UTC according to the standard. The DATE header is mostly irrelevant to data processing since it it just a statement about HDU creation and not a statement about arrival time of photons.

            tjenness Tim Jenness added a comment - (1) is working as designed. All DATE-* headers use TIMESYS which is set to TAI. The DATE header is always UTC according to the standard. The DATE header is mostly irrelevant to data processing since it it just a statement about HDU creation and not a statement about arrival time of photons.

            As Angelo said, the timestamps coming back from the time series queries should be in TAI since they are just being forwarded from the influxDB index (which currently are TAI). This will change when we start using UTC for everything. If they need to be adjusted to agree with other TAI timestamps, that points to an error in the timestamps provided by influxDB.

            As an aside, you can avoid calling merge_packed_time_series by just calling select_packed_time_series to start with. The stated caveats above still apply.

            krughoff Simon Krughoff (Inactive) added a comment - As Angelo said, the timestamps coming back from the time series queries should be in TAI since they are just being forwarded from the influxDB index (which currently are TAI). This will change when we start using UTC for everything. If they need to be adjusted to agree with other TAI timestamps, that points to an error in the timestamps provided by influxDB. As an aside, you can avoid calling merge_packed_time_series by just calling select_packed_time_series to start with. The stated caveats above still apply.
            tjenness Tim Jenness added a comment -

            krughoff is there a ticket that we can link to this one to indicate this ticket is blocked?

            tjenness Tim Jenness added a comment - krughoff is there a ticket that we can link to this one to indicate this ticket is blocked?

            Today we implemented DM-30083 and all instances of the EFD are now recording data in UTC.

            The EFD client version 0.9.0 should be used when querying new data in UTC.

            This version also implementes a compatibility mode to query old data in TAI which seems to have a bug. We are working on DM-32225 to fix that.

            afausti Angelo Fausti added a comment - Today we implemented DM-30083 and all instances of the EFD are now recording data in UTC. The EFD client version 0.9.0 should be used when querying new data in UTC. This version also implementes a compatibility mode to query old data in TAI which seems to have a bug. We are working on DM-32225 to fix that.

            cslage thanks for the help in validating this change. You'll notice that the default version of the EFD client is still 0.8.3 on nublado. To install 0.9.0, you can open a Terminal and do:

            pip install --user lsst_efd_client==0.9.0

            

            then restart the LSST kernel in the notebook to pick the new version. You should see:

            import lsst_efd_client
            print(lsst_efd_client.__version__)
            __version__ = '0.9.0'
            

            afausti Angelo Fausti added a comment - cslage thanks for the help in validating this change. You'll notice that the default version of the EFD client is still 0.8.3 on nublado. To install 0.9.0, you can open a Terminal and do: pip install --user lsst_efd_client==0.9.0
 then restart the LSST kernel in the notebook to pick the new version. You should see: import lsst_efd_client print(lsst_efd_client.__version__) __version__ = '0.9.0'
            cslage Craig Lage added a comment - - edited

            Thanks afausti. I'm setting up try to take the images today, and hopefully can finish the analysis by tomorrow.

            I took a set of images and did a quick analysis. The notebook I used is at NCSA at /project/cslage/AuxTel/notebooks/Plot_Tracking_UTC_14Oct21.ipynb
            Here is what I concluded:

            (1) In the fits headers, the DATE keyword is still in UTC and the DATE_BEG and DATE_END keywords are still in TAI. I think this is as intended.

            (2) I only checked a few timestamps from the regular time series, but lsst.sal.ATMCS.logevent_allAxesInPosition and
            lsst.sal.ATCamera.logevent_shutterDetailedState
            are definitely now in UTC.

            (3) It appears that the merged_packed_time_series are still returning data in TAI. I queried for data between 18:48:29 and 18:48:39 ,and it returned a dataFrame starting at 18:49:06, 37 seconds later. See plot Tracking_Timebase_2021101400011_14Oct21.pdf. If I shift the query by 37 seconds, the plot (Tracking_Timebase_2021101400011_TAI_Shift_14Oct21.pdf) all makes sense.

            Let me know if more tests are needed.

            cslage Craig Lage added a comment - - edited Thanks afausti . I'm setting up try to take the images today, and hopefully can finish the analysis by tomorrow. I took a set of images and did a quick analysis. The notebook I used is at NCSA at /project/cslage/AuxTel/notebooks/Plot_Tracking_UTC_14Oct21.ipynb Here is what I concluded: (1) In the fits headers, the DATE keyword is still in UTC and the DATE_BEG and DATE_END keywords are still in TAI. I think this is as intended. (2) I only checked a few timestamps from the regular time series, but lsst.sal.ATMCS.logevent_allAxesInPosition and lsst.sal.ATCamera.logevent_shutterDetailedState are definitely now in UTC. (3) It appears that the merged_packed_time_series are still returning data in TAI. I queried for data between 18:48:29 and 18:48:39 ,and it returned a dataFrame starting at 18:49:06, 37 seconds later. See plot Tracking_Timebase_2021101400011_14Oct21.pdf. If I shift the query by 37 seconds, the plot (Tracking_Timebase_2021101400011_TAI_Shift_14Oct21.pdf) all makes sense. Let me know if more tests are needed.
            tjenness Tim Jenness added a comment -

            cslage thanks for checking. (1) Yes. (2) Excellent.

            I have no idea about (3).

            tjenness Tim Jenness added a comment - cslage thanks for checking. (1) Yes. (2) Excellent. I have no idea about (3).
            cslage Craig Lage added a comment - - edited

            I think (3) is clearly wrong. It should not return a dataFrame that starts 37 seconds before after the beginning of the query. But I don't know if the problem is with the code or with the data stored in the EFD. And I don't know who the right person to track it down is. krughoff, do you know who can track this down?

            cslage Craig Lage added a comment - - edited I think (3) is clearly wrong. It should not return a dataFrame that starts 37 seconds before after the beginning of the query. But I don't know if the problem is with the code or with the data stored in the EFD. And I don't know who the right person to track it down is. krughoff , do you know who can track this down?

            The time index in influxDB was intentionally stored in TAI because that is how it is recorded in the dome. Unfortunately, Pandas has no concept of TAI, so we have to lie about the scale in the dataframe. We have now migrated all influxDB instances to use UTC as the time index, but you will have to use v 0.0.9 of the lsst_efd_client to use those. I think that will clear some things up. I'm sorry this has been confusing and is costing you time, but we'll get this sorted out.

            krughoff Simon Krughoff (Inactive) added a comment - The time index in influxDB was intentionally stored in TAI because that is how it is recorded in the dome. Unfortunately, Pandas has no concept of TAI, so we have to lie about the scale in the dataframe. We have now migrated all influxDB instances to use UTC as the time index, but you will have to use v 0.0.9 of the lsst_efd_client to use those. I think that will clear some things up. I'm sorry this has been confusing and is costing you time, but we'll get this sorted out.
            cslage Craig Lage added a comment - - edited

            I think you mean v0.9.0. This is what I used, as described by Angelo Fausti above, and I verified in the notebook that v0.9.0 had been loaded, but the problem with merged_packed_time_series is still there. Regardless of the scale, the query should not return data that starts 37 seconds before after the start of the query. Can't we just add an offset within the merged_packed_time_series and select_packed_time_series methods so that it returns data within the query? By the way, no worries about my time. For health reasons, I have lots of time on my hands, and am just trying to help get this cleaned up.

            cslage Craig Lage added a comment - - edited I think you mean v0.9.0. This is what I used, as described by Angelo Fausti above, and I verified in the notebook that v0.9.0 had been loaded, but the problem with merged_packed_time_series is still there. Regardless of the scale, the query should not return data that starts 37 seconds before after the start of the query. Can't we just add an offset within the merged_packed_time_series and select_packed_time_series methods so that it returns data within the query? By the way, no worries about my time. For health reasons, I have lots of time on my hands, and am just trying to help get this cleaned up.

            OK. I missed that you were using the new client. The query should return samples within the query bounds. If there is an offset, there is a misunderstanding someplace. FWIW, I never meant merge_packed_time_series to be used directly. I expected people to use select_packed_time_series since that does the same thing in one step instead of two. If you find you need to use it directly, I'm curious why.

            Which notebook should I use to see the issue you are reporting?

            krughoff Simon Krughoff (Inactive) added a comment - OK. I missed that you were using the new client. The query should return samples within the query bounds. If there is an offset, there is a misunderstanding someplace. FWIW, I never meant merge_packed_time_series to be used directly. I expected people to use select_packed_time_series since that does the same thing in one step instead of two. If you find you need to use it directly, I'm curious why. Which notebook should I use to see the issue you are reporting?
            cslage Craig Lage added a comment -

            OK, I converted the notebook to only use select_packed_time_series. I agree, it is simpler. The problem is still there, since it's still the same code. You should be able to see it at NCSA at /project/cslage/AuxTel/notebooks/Plot_Tracking_UTC_20Oct21.ipynb. Let me know if you can't access it and I'll upload it somewhere.

            cslage Craig Lage added a comment - OK, I converted the notebook to only use select_packed_time_series. I agree, it is simpler. The problem is still there, since it's still the same code. You should be able to see it at NCSA at /project/cslage/AuxTel/notebooks/Plot_Tracking_UTC_20Oct21.ipynb. Let me know if you can't access it and I'll upload it somewhere.

            cslage thanks very much for the pointer to the notebook, it was a major help! It turns out that I neglected to deal with the fact that the time series that we store that are not the message index are still in TAI. The timestamp used in the case where the time series is the packed version can be any of the internal ones, so I needed to make sure to convert those, if necessary. I'll have a fix out in the next day or so.

            krughoff Simon Krughoff (Inactive) added a comment - cslage thanks very much for the pointer to the notebook, it was a major help! It turns out that I neglected to deal with the fact that the time series that we store that are not the message index are still in TAI. The timestamp used in the case where the time series is the packed version can be any of the internal ones, so I needed to make sure to convert those, if necessary. I'll have a fix out in the next day or so.
            cslage Craig Lage added a comment -

            krughoff - Great! Let me know, and I'll test it again. Then we should be able to close this ticket.

            cslage Craig Lage added a comment - krughoff - Great! Let me know, and I'll test it again. Then we should be able to close this ticket.

            FWIW, I'm going to put the fix on the DM-32225 branch since I'm fixing another bug there already. I'll keep the commits separate.

            krughoff Simon Krughoff (Inactive) added a comment - FWIW, I'm going to put the fix on the DM-32225 branch since I'm fixing another bug there already. I'll keep the commits separate.
            cslage Craig Lage added a comment -

            krughoff, can I pull the tickets/DM-32225 branch of lsst-sqre/lsst-efd-client, and test it out, or are you still discussing the best fix? There's an AuxTel observing run next week, and I need to fix up some code that tracks mount tracking errors before then, so I'd like to move ahead. Thanks!

            cslage Craig Lage added a comment - krughoff , can I pull the tickets/ DM-32225 branch of lsst-sqre/lsst-efd-client, and test it out, or are you still discussing the best fix? There's an AuxTel observing run next week, and I need to fix up some code that tracks mount tracking errors before then, so I'd like to move ahead. Thanks!

            cslage sorry for the delay. I'm building a new version right now that incorporates the fixes from DM-32225. It's on PyPI now. Here is the plot from you notebook with the 0.9.1 version and it looks right to me.

            One side note, I notice you use a hard coded 37 in parts of your notebook to convert between UTC and TAI. I would strongly suggest you create one astropy.Time object and use it to convert with the .utc and .tai attributes. I can give you examples of how I modified your notebook to do that.

            krughoff Simon Krughoff (Inactive) added a comment - cslage sorry for the delay. I'm building a new version right now that incorporates the fixes from DM-32225 . It's on PyPI now. Here is the plot from you notebook with the 0.9.1 version and it looks right to me. One side note, I notice you use a hard coded 37 in parts of your notebook to convert between UTC and TAI. I would strongly suggest you create one astropy.Time object and use it to convert with the .utc and .tai attributes. I can give you examples of how I modified your notebook to do that.

            The ticket with the fix is DM-32225 and is marked Done, but I'm going to hold off marking this done until Craig can confirm things now work as he expects.

            krughoff Simon Krughoff (Inactive) added a comment - The ticket with the fix is DM-32225 and is marked Done, but I'm going to hold off marking this done until Craig can confirm things now work as he expects.
            cslage Craig Lage added a comment -

            krughoff Yes, I was unhappy with hard-coding the 37 seconds. Please show me how to do this with astropy.

            cslage Craig Lage added a comment - krughoff Yes, I was unhappy with hard-coding the 37 seconds. Please show me how to do this with astropy.
            cslage Craig Lage added a comment -

            krughoff I got the same plot as you did with the new code. It bothers me because the "All Axes In Position" timestamp should not occur until the mount has stabilized, and you can see that the mount is still moving at that point. It looks as though the packed time series is shifted to the right by a small amount (1-2 seconds?) relative to the others. In the past there has been discussion of an unexplained 2 second shift in the cRIO_timestamp. Could that still be there? I'll try to dig up the ticket that mentions this.

            cslage Craig Lage added a comment - krughoff I got the same plot as you did with the new code. It bothers me because the "All Axes In Position" timestamp should not occur until the mount has stabilized, and you can see that the mount is still moving at that point. It looks as though the packed time series is shifted to the right by a small amount (1-2 seconds?) relative to the others. In the past there has been discussion of an unexplained 2 second shift in the cRIO_timestamp. Could that still be there? I'll try to dig up the ticket that mentions this.

            If there is a small shift in the cRIO_timestamp, the client would not be able to take care of that

            krughoff Simon Krughoff (Inactive) added a comment - If there is a small shift in the cRIO_timestamp, the client would not be able to take care of that

            Here is the notebook I based on yours. The two changes I made were to use astropy.Time to do the conversion between UTC and TAI (cell 5), and I modified one query to use select_time_series instead of constructing the query by hand (cell 8). I think the convenience functions should be used as much as possible because it makes sure the queries are all formatted similarly and it also populates the query_history buffer which can help in debugging. Plot_Tracking_UTC_20Oct21.ipynb

            krughoff Simon Krughoff (Inactive) added a comment - Here is the notebook I based on yours. The two changes I made were to use astropy.Time to do the conversion between UTC and TAI (cell 5), and I modified one query to use select_time_series instead of constructing the query by hand (cell 8). I think the convenience functions should be used as much as possible because it makes sure the queries are all formatted similarly and it also populates the query_history buffer which can help in debugging. Plot_Tracking_UTC_20Oct21.ipynb
            cslage Craig Lage added a comment -

            tjennessSo I think we consider this ticket Done. Any further improvements will need to come from changes to the cRIO_timestamp in the EFD. Brian has created a separate ticket (https://jira.lsstcorp.org/browse/CAP-816) for that.

            cslage Craig Lage added a comment - tjenness So I think we consider this ticket Done. Any further improvements will need to come from changes to the cRIO_timestamp in the EFD. Brian has created a separate ticket ( https://jira.lsstcorp.org/browse/CAP-816 ) for that.
            tjenness Tim Jenness added a comment -

            Okay. I've marked it done.

            tjenness Tim Jenness added a comment - Okay. I've marked it done.

            People

              krughoff Simon Krughoff (Inactive)
              cslage Craig Lage
              Craig Lage
              Angelo Fausti, Craig Lage, Felipe Menanteau, Frossie Economou, Merlin Fisher-Levine, Patrick Ingraham, Simon Krughoff (Inactive), Steve Pietrowicz, Tiago Ribeiro, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.