Units problems in psf-quicksim.fits

[dkirkby]

(I know this file is actually in SVN rather than this repo, but thought an issue here might be more useful than a trac ticket).

astropy.io.fits is flagging some units problems when reading desimodel/data/specpsf/psf-quicksim.fits:

WARNING: UnitsWarning: The unit 'Angstrom' has been deprecated in the FITS standard. Suggested: 10**-1 nm. [astropy.units.format.utils]
WARNING: UnitsWarning: 'Angstrom/row' did not parse as fits unit: At col 9, Unit 'row' not supported by the FITS standard.  [astropy.units.core]

These are harmless for now, but should be easy to fix the next time someone updates this file.

[sbailey]

For the record, this has the feeling of astropy trying to irritate us into doing something formally right, but somewhat less useful than what we are currently doing.

"Angstrom" is clearer at a glance than "10**-1 nm". And Angstroms are the defacto optical astronomy wavelength unit at a useful scale, even if they aren't SI. Using Angstrom may cause astropy warnings, but it doesn't cause confusion (though the astropy warnings themselves might cause confusion).

"Angstrom/row" is more questionable, but it does seem odd to have a variable whose name is "angstroms_per_row" and not have its units also be "Angstrom/row". The real unit of this quantity is Angstrom/row — it is the derivative of the wavelength with respect to CCD row index. The unit is not just Angstrom, so if we do change it, don't change it to that.

[weaverba137]

PS, yes, this is the correct place to report all problems with desimodel!

[dkirkby]

@sbailey I agree with all that. I just wanted to flag the issue since these warnings are now printed by specsim after I switched from astropy.io.fits.open() to astropy.table.Table.read() for reading the PSFs. It looks like they are using the warnings module, so we can probably silence this particular message.

For Angstrom/row, we could define a custom dimensionless unit, e.g.

>>> row_unit = u.NamedUnit('row')
>>> print((3 * u.Angstrom) / (2 * row_unit))
1.5 Angstrom / row

[dkirkby]

I ended up with a different solution to this problem in specsim: I use Angstrom/pixel for the units instead of Angstrom/row. This works because pixel is already a pre-defined astropy unit. You could quibble that rows and pixels are not the same, but astropy does not specify whether a pixel is a linear unit or an area unit. I use it consistently as a linear unit now in specsim, and everything works nicely. This means, for example, that the dark current now has units of electron/(hour pixel**2).

Of course, we will still get the UnitsWarning messages when reading psf-quicksim.fits until the units are updated in that file.

[weaverba137]

Is this still an issue for desimodel, or is does the workaround completely fix the problem?

[dkirkby]

There is still a non-critical action item to replace row with pixel in the FITS table units of psf-quicksim.fits.

[weaverba137]

Can we please resolve this issue so the data model for these files can be finalized?

[dkirkby]

@sbailey is the owner of the code that generates psf-quicksim.fits but this issue is not blocking anything as far as I know. This might be a case where the datamodel should document the current units, and then we will update the datamodel when / if the units are eventually changed.

[weaverba137]

Adding label 'data model' note that fixing this issue will also involve changes to the data model.