Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritize date_start when defining WCS and coordinate frame #98

Merged

Conversation

wtbarnes
Copy link
Contributor

As pointed out by @yjzhu-solar, in EIS SW Note 9, Section 2.4, it states that,

The FITS headers, however, do not include the time of each exposure, and thus the pointing keywords should be tied to a time that is also provided in the header. The best time to use is the one contained in the DATE OBS keyword, the start time of the raster. This means that all the pointing data in the FITS headers need to be positions at that time.

Previously, EISMap was overriding .date in order to prioritize date_average, the midpoint of the raster, clearly in contrast to the above statement about how the pointing keywords are defined. As such, this PR does two things:

  1. Removes the overriding of .date. Now, EISMap.date will fall back to that in sunpy.map.GenericMap which will prioritize DATE-OBS and DATE-BEG before DATE-AVG.
  2. Adds a reference_date property which always prioritizes date_start. (see below for rationale of why this was added).

This fix is somewhat complicated by the fact that date handling in sunpy was recently overhauled (see sunpy/sunpy#7682 for more information). However, with these fixes, EISMap should still use the correct date in both current and future versions of sunpy:

  • In versions prior to sunpy v6 (i.e. 5.1.x and below), .date is used to to define the WCS as well as coordinate frame. As such, with this PR, .date_start will now be used to define the WCS and coordinate frame.
  • In version 6.0 of sunpy and beyond, .reference_date is used to define the coordinate frame. Additionally, on the WCS, the dateavg property is set to to .reference_date such that the start date of the raster is appropriately prioritized.

I've also adjusted and simplified the tests for the date handling in EISMap as most of that testing is now covered in GenericMap anyway.

Regarding the definitions of the EIS pointing keywords, two questions:

  1. That SW note is rather old. Is this all still true?
  2. That SW pertains to the full raster files. Does it still hold for the FITS files that are produced after fitting? I'm not sure why it wouldn't be, but figured I would ask the question.

@yjzhu-solar
Copy link

In addition, it seems that the solar rotation compensation is not always on. I did not find helpful information in the FITS header indicating whether the solar rotation is compensated, except for ASRCDIR = 0 /Anti Solar Rot. Comp. dir. (0=forw, 1=backw). I am going to reach out to more EIS team members for help.

@MJWeberg
Copy link
Collaborator

Great questions! We do have all of the pointing and timing information in the EIS level-1 HDF5 files, which are loading into .meta['date_obs'] and .meta['pointing'] in the EISCube objects (and copied over to the the meta information in the full EISFitResults object). I did some tests today and can confirm that, yes, the pointing information we are saving in EISPAC is consistent with the corrected coordinates given in the SSW routines. Specifically, the coordinates are given for the full EIS FoV as seen at the start of the raster (date_obs of the first exposure). Therefore this PR should give the correct coords using SunPy maps.

As to @yjzhu-solar's point about solar rotation, I am told that adjustments for rotation are done on the spacecraft side, rather than the instrument side. Almost all on-disk rasters should be taken while the spacecraft actively tracking and adjusting for rotation while off-limb observations are typically taken without tracking. The tracking mode is indicated by the tr_mode key in the original fits header (or the .meta['index'] in an EISCube). As best as I can tell, "FIX" and "TR0" indicate no tracking while "TR1" - "TR4" indicate different tracking modes (not sure at the moment what each mode means). Unfortunately, the tr_mode keyword is not currently exported the EISMap or ".fits" files saved by EISPAC, but this can be corrected easily.

Thanks for catching this! Since correcting the default timestamps may affect coordinate conversions to/from an EISMap, do you think we should raise an warning in the console to notify users of the change?

@wtbarnes
Copy link
Contributor Author

Thanks for catching this! Since correcting the default timestamps may affect coordinate conversions to/from an EISMap, do you think we should raise an warning in the console to notify users of the change?

It's a fair point that this is a breaking change, but I would claim this is really a bugfix and is the way it should've worked from the start (I really should have read the accompanying software note before assuming these were defined at date_avg!). If you want to make users aware of this change, I would stick a note in the changelog or release notes for whatever release this makes it into.

Comment on lines +138 to +140
.. note:: This property is overridden because `sunpy.map.GenericMap` sets
this to be the ``.date_average`` which in this case is the midpoint
of the raster.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the syntax for this?

Suggested change
.. note:: This property is overridden because `sunpy.map.GenericMap` sets
this to be the ``.date_average`` which in this case is the midpoint
of the raster.
.. note::
This property is overridden because `sunpy.map.GenericMap` sets
this to be the ``.date_average`` which in this case is the midpoint
of the raster.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe either works

eispac/tests/test_eismap.py Show resolved Hide resolved
@yjzhu-solar
Copy link

As best as I can tell, "FIX" and "TR0" indicate no tracking while "TR1" - "TR4" indicate different tracking modes (not sure at the moment what each mode means). Unfortunately, the tr_mode keyword is not currently exported the EISMap or ".fits" files saved by EISPAC, but this can be corrected easily.

Thank you for pointing out the correct FITS keyword! Should we determine the timestamp to use based on this keyword? Is it better to issue a warning about the solar rotation tracking status and the decision on which timestamp to use?

@MJWeberg MJWeberg merged commit 3ac2abb into USNavalResearchLaboratory:main Sep 3, 2024
4 checks passed
@MJWeberg
Copy link
Collaborator

MJWeberg commented Sep 3, 2024

Finally merging this in and will start work on updating the fits header for better compliance with the FITS-4 standard (see #100) and include more spacecraft- and instrument-specific keywords (such as the tr_mode key mentioned here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants