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

the unit 'nmgy' could not be saved in native FITS format #72

Closed
moustakas opened this issue Jul 15, 2022 · 7 comments
Closed

the unit 'nmgy' could not be saved in native FITS format #72

moustakas opened this issue Jul 15, 2022 · 7 comments

Comments

@moustakas
Copy link
Member

#69 propagates units through all the astropy tables using the QTable object, but when writing the files in the last step, the following warning is issued. Not sure how to deal with this at the moment but tagging @weaverba137 @geordie666 @dstndstn @sbailey as there's a similar discussion (I think?) going on with the imaging / targeting files.

WARNING: The unit 'nmgy' could not be saved in native FITS format and cannot be recovered in reading. It can roundtrip within astropy by using QTable both to write and read back, though one has to enable the unit before reading. [astropy.io.fits.convenience]
WARNING: The unit '1 / nmgy2' could not be saved in native FITS format and cannot be recovered in reading. It can roundtrip within astropy by using QTable both to write and read back, though one has to enable the unit before reading. [astropy.io.fits.convenience]
@moustakas
Copy link
Member Author

Actually, I've decided to abandon the astropy.table.QTable object, which affect performance noticeably and, even worse, when you astropy.table.vstack a list of QTables the units aren't preserved! So although the tables are seeded with units, by the time the parallelism has gone through and all the tables are stacked together, the units (in columns which have them) are lost, which is frustrating. I can think of one fix but it's clunky; other suggestions are welcome.

@moustakas
Copy link
Member Author

moustakas commented Jul 15, 2022

Lots of relevant discussion in astropy/astropy#7279, legacysurvey/legacysurvey#158, and elsewhere.

@weaverba137
Copy link
Member

When you say a warning is issued, does it still physically write the unit to the file?

Also note 1 / nmgy2 is invalid. It should be nmgy-2.

@moustakas
Copy link
Member Author

moustakas commented Jul 15, 2022

When you say a warning is issued, does it still physically write the unit to the file?

No, astropy issues the warning and doesn't write any units. However, reading through some of the tickets linked above, I ended up writing the unit as a custom 'nanomaggies' unit and that works without any warnings.

Also note 1 / nmgy2 is invalid. It should be nmgy-2.

Well, astropy issues this warning, so they'll need to clean that up.

@weaverba137 @sbailey if either of you have cycles to review the headers of the files written by #69 (and the data model, if you're inspired), I would be grateful as I'm getting close to launching the fits to Fuji & Guadalupe. These catalogs will end up as an EDR VAC, so it makes sense to have the files be reviewed by the Data Team (plus the fitting is expensive, so I'd prefer not to have to redo it too many times!).

The two main files / file types (for just 2 objects/spectra) can be found here:

/global/cscratch1/sd/ioannis/fastspecfit-datamodel/fastphot.fits
/global/cscratch1/sd/ioannis/fastspecfit-datamodel/fastspec.fits.gz

Note that the fastspec file is intentionally gzipped (and the fastphot file is not) because it contains a third HDU containing the model spectra.

Please let me know if you identify any issues and remember that in production I'll use a consistent set of tags between fastspecfit and its dependencies. Thanks!

@weaverba137
Copy link
Member

@moustakas, I'm saying that 1 / anything2 is invalid, no matter the validity of nmgy. That one you must fix, it's not just astropy being fussy.

@moustakas
Copy link
Member Author

@moustakas, I'm saying that 1 / anything2 is invalid, no matter the validity of nmgy. That one you must fix, it's not just astropy being fussy.

None of my header cards have this form and never did, even when I was using u.nmgy. I'm saying that the astropy warning (and astropy itself) is wrong

>>> import astropy.units as u
>>> u.nmgy**(-2)
  Unit("1 / nmgy2")

But regardless, I'm not using this unit anymore; check out the headers of my files when/if you get a chance.

@geordie666
Copy link

I believe that astropy is internally consistent, it's just that the FITS standard for units is different from the astropy standard, so you have to be careful to format for FITS when writing FITS headers.

>>> import astropy.units as u
>>> u.deg**(-2)
  Unit("1 / deg2")
>>> u.Unit('deg**(-2)', format='fits')
  Unit("1 / deg2")
>>> u.Unit('1 / deg2', format='fits')
  ValueError: '1 / deg2' did not parse as fits unit

This isn't obvious. I certainly ran afoul of it when writing units for target files, and some of those therefore have 1 / deg2 for file units (e.g., see the note at the bottom of the data model for the target files).

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

No branches or pull requests

3 participants