units: units in fits file does not parse - #1911

The unit parsers in astropy are based on the grammars provided by the unity library [1], which came out of the IVOA's VOUnit project to unify Unit syntaxes. That's where the strict insistence on a single solidus comes from.

[1] http://www.astro.gla.ac.uk/users/norman/ivoa/unity/c-docs/html/index.html

We've discussed case sensitivity elsewhere and have #1870 in the pipeline, but that won't fully address the issue of just loading a FITS file.

My general view on this in other areas of astropy is that users should know when their files are not written to a standard, because it inhibits interoperability with other tools. In that light it's probably time to introduce a "relaxed" mode on FITS units, which could be accessed through the existing fix functionality of astropy.io.fits.

I'm going to stop you slightly there because I have plans to overhaul PyFITS/astropy.io.fits validation handling. I was hoping to have a new hire work on that, but hiring continues to drag on so...

That said, there will still be a similar functionality to allow fixing fixable validation errors, so maybe I don't need to stop you there at all.

I feel like the parser should remain strict--it should return an exception. But is it possible for the parser to figure out what the user meant, or should that be handled on the FITS validation end upon catching an exception in unit parsing?

The FITS standard should not impose a requirement that is contrary to generally accepted usage. Check the AIP style guide, for example. The name of a unit should not be capitalized (e.g. watt, not Watt). Abbreviations for units should be capitalized if and only if (except L is OK for liter) the unit is a person's name (e.g. W).

Perhaps; but the FITS standard does exactly that in a lot of ways. Of course we could try to get that changed but it would take...months? Years? And then all the FITS files that do follow the standard on this admittedly minor issue would be invalid. So much for "once FITS always FITS" (which itself has almost never been quite true...)

@philhodge: We have to work with the standard we have, not the one we want, at least until such time that we've worked to change the standard (which as @embray points out is epic), or replaced it with a new one (which the VOUnit initiative aims to do).

I think part of the problem here is that so many tools have treated units as opaque strings without much attempt to interpret them, so these sorts of incompatibilities go unchecked...

Strangely, the VOUnit document, which serves as a pretty good survey of all things in astronomical units, lists "Angstrom" as deprecated for the FITS format (without any citation, so I'm not sure where they get that from, given that FITS 3.0 lists it explicitly), and it lists "angstrom" as never having been accepted.

I think that the "deprecation" they're referring to is a footnote in the FITS Standard on Angstrom (as well as G (for gauss), erg, and barn):

"Deprecated in IAU Style Manual (McNally 1988) but still in use."

I think probably this goes back to "once FITS always FITS". I don't think the usage is actually deprecated in the FITS format itself, as no alternative is provided.

On 12/18/2013 01:43 PM, Erik Bray wrote:

Perhaps; but the FITS standard does exactly that in a lot of ways. Of
course we could try to get that changed but it would take...months?
Years? And then all the FITS files that do follow the standard on this
admittedly minor issue would be invalid. So much for "once FITS always
FITS" (which itself has almost never been quite true...)

You don't have to raise an exception just because the unit is written
"angstrom" instead of "Angstrom". IMHO a warning would be sufficient,
if not overkill.

Well, in the case of reading/writing actual FITS files I agree, and that's part of what I've been wanting to overhaul in the validation handling. I want to explicitly classify all validation issues between errors, warnings, and perhaps a category for issues that, while they do technically violate the standard, are little more than cosmetic.

But the units parser I think should be strict and should raise an exception. However the exception should contain enough information for a FITS validator to catch it and understand what went wrong and handle it--in most cases by raising a warning, fixing the issue, or possibly taking no action at all.

And just to clear this up (since @hamogu's original post doesn't really address it), the current behavior is that unparsable units found while reading a file raise warnings, not exceptions, so that behavior already agrees with what @philhodge is suggesting. I'm not advocating changing that, only that if we decide to veer from the spec to try to guess the intent of the FITS file, that should be optional.

I agree that we don't want to guess too much. However, guessing where it's unambiguous (Angstrom vs. angstrom) would make the software more usable.
If it does not parse the units, my first reaction is: "Well, if that does not work even for an HST fits file, what is all this unit stuff good for anyway?"
It's only because I am contributing to astropy that I thought about opening an issue for that. If the same had happened in any other library I probably would have ignored the warning and dismissed the package as "well, it's just not working".

As a second point, to come back to the original issue, using multiple "/" is explicitly declared valid (although discouraged) in the fits standard. So, that should work for strict and loose interpretation of units.

I've gone ahead and implemented parsing for multiple "/", since we probably should support that somehow, however, the interpretation is ambiguous. This is from the VOUnit spec:

the FITS specification isn’t completely clear
on the topic of solidi, saying “[t]he IAU style manual forbids the use of more
than one solidus (/) character in a units string. However, since normal
mathematical precedence rules apply in this context, more than one solidus
may be used but is discouraged”. This does not really resolve the question of
whether, for example, kg/m s should be parsed as "kg m-1 s-1" or "kg m-1 s",
since there is a question of both operator precedence and (left-)associativity,
where there might be different rules internationally, and conflicts between
mathematical and programming-language rules.  Most people would probably
parse it as "kg m-1 s-1", but we trust that most educators would oblige
students to rewrite the expression on the grounds that any ambiguity is too
much.  Here, we resolve the ambiguity by declaring that there can be only a
single expression to the right of the solidus.

So, my implementation here treats things as "km m-1 s-1", but also raises a big warning when using multiple slashes. This seems to agree with what wcslib does when parsing units. Note that the implementation will never write out a (parsed) unit string with multiple slashes.

As for angstrom, I've added it to the list of "deprecated" units in FITS. It isn't technically deprecated, since it was never in the standard, but it is in common usage (in HST) though it's future use should be discouraged. This uses the existing support for deprecated FITS units which will raise a warning when they are used.

The reason I wrote the units for flux as "erg /s /cm2 /angstrom" was the recommendations in the paper "Specification of Physical Units within OGIP FITS files", by Ian M George & Lorella Angelini. I was reading the 1993 Aug 06 version, but the current version, 1995 May 04, is similar and what I quote below. (OGIP is the Office of Guest Investigators Program for high-energy astrophysics.) Table 2 lists "angstrom" (not "Angstrom"). In the examples, example 3 is: STRING = 'count /m2 /s /eV' (recommended).

The great thing about standards is that there are so many of them. ;) So in fact the units in question are not based on the FITS standard, but the OGIP conventions, which are considerably different from the FITS standard in a number of ways, including the names of the units themselves and the operators (OGIP doesn't support . as a multiplier, for example). I haven't implemented the OGIP conventions in astropy yet, but maybe now is the time to bite the bullet and do that. The difficulty here is there is no defined way to determine from the FITS file which convention is being used. And the two formats are fundamentally incompatible due to how slashes are handled. The VOUnit folks have this to say:

It is a consequence of this [multiple solidus ambiguity] that nothing can
be successully parsed in two
different grammars, with different meanings. If the right-hand-side of the
division could be a product_of_units, then `kg /m s` would parse in both
the FITS and OGIP syntaxes, but mean `kg m-1 s-1` in FITS and `kg m-1 s`
in the OGIP one.

(since in OGIP, the '/' only affects the unit immediately after it).

In the case of the original example here, erg /s /cm**2 /angstrom parses identically in both the FITS and OGIP conventions, so we "got lucky".

Therefore (and I think this is getting out of the scope of this single PR), I think ultimately what we need to do here is:

• Implement the OGIP unit standard in astropy
• Provide a way to pass which units to expect in the file to fits.open etc. (@embray thoughts)
• Optionally, automatically guess which unit convention might be in use by looking at other keywords in the header (I have no idea if there's a way to do that -- the OGIP standard doesn't seem to address it, so this would just be a bunch of real-world heuristics, such as detecting the HST files that contain these units. There isn't any way, as far as I can tell, to reliably determine that from the unit string itself.)

Side rant: who thought that it would be a good idea to have kg/m s parse as kg/m/s, against all rules of arithmetic learnt in primary schools? Who writes these standards!?

On Dec 19, 2013, at 10:48 AM, Thomas Robitaille wrote:

Side rant: who thought that it would be a good idea to have kg/m s parse as kg/m/s, against all rules of arithmetic learnt in primary schools? Who writes these standards!?

In any case, given the different uses actually appearing in FITS files, I think one has to conclude that units in FITS is effectively broken. One more item to pass to the ongoing FITS paper I think.

In a (non-representative) survey of fits files I happen to be working on right now, it seems that most of them parse unambiguously.
It's also easy to test (once OGIP is implemented) if OGIP and fits paste a unit the same way, so I would prefer a "relaxed" mode that "just works" for units that parse the same in both standards.

@mdboom I understand that you want to alert users if there files are not standard complaint so that they complain and eventually get the software that makes the files fixed. In the case of fits that's never going to happen (for all the reasons mentioned above and elsewhere). So, unlike e.g. in vofiles the justification to "alert the user to every violations of the standard" is not very important here and, as long as we have fits (which will be a long time), a "just works" for all unambiguously parsing files would be great.

If we're to support OGIP units at all I would suggest devising some heuristics rather than allowing an explicit argument. I would consider adding a flag for enabling/disabling handling of such units, but the default always needs to be "enabled". Yep, another good item to pass on the "Why FITS is failing..." paper or whatever it's called today.

Just for reference (some of the people on this list may have been involved from the other side), here is the official answer I received from the STScI helpdesk, where I reported this problem:

Dear Dr. Günther,

On behalf of the COS/STIS Help Desk I would like to thank you for pointing this [the non-complience with the fits standard for the HST/COS file in question] out to us. This information will be provided to the responsible individuals to determine what course of action should be taken.

Thank you very much for your time.

Best Regards,

Kevin Lindsay
COS/STIS Help Desk

@embray: So let's assume we add a third "parsing" mode by default which attempts to handle both the FITS and OGIP unit formats through some sort of heuristic. As far as unit names go, there isn't really anything contradictory, so we can just accept the union of them. The issue is with the handling of the slash. When multiple slashes are used, but no multiplication is found after the first slash the two are equivalent. However, if a slash is followed by a multiplication, we'll have to throw up our hands, since it's ambiguous. I don't know how common that case is in practice. But that suggests to me we still need some way for the user specify which kind of units they are expecting to find and force an interpretation of the slash character -- or maybe we can determine that automatically from the source of the FITS file (determined from some other keyword), but we'll probably never have a complete database of how to interpret units given the source of the FITS file.

Note also I think we can merge this PR without resolving the question about how to automatically handle FITS vs. OGIP units, as this makes sense to do independently of that.

I came across this discussion at the end of last week (I'm one of the VOUnit authors). Thanks for the various observations here.

@mdboom and @embray: yes, the 'deprecations' listed for FITS are indeed based on that footnote to the FITS spec's table 4. I've clarified this in the repository version of the document.

I've also clarified that the although the unit name is indeed 'angstrom' (lower-case initial) the 'preferred' symbol is 'Angstrom' over 'angstrom', following the pattern of other symbols named after surnames, and following FITS and CDS, and against OGIP.

Also, @mdboom mentions that "[...]replaced it with a new one (which the VOUnit initiative aims to do)." Our position on that is that the VOUnits spec wasn't intended to replace anything, as such, but that (a) VO specs had to have something to refer to as an allowed syntax in unit contexts, and (b) none of the existing syntaxes was universally adopted, or completely unambiguous, with a grammar. The goal was that the VOUnits syntax would be as much as possible a superset of the three pre-existing common standards. This last isn't quite possible in practice; section 2.12 of the document discusses this.

I've created a page of VOUnits errata-ettes.

Thanks again for the notes which prompted these.