Starlink / starlink

Starlink Software Collection
160 stars 53 forks source link

FITS2NDF creates SMURF extension with the wrong type #60

Open grahambell opened 7 years ago

grahambell commented 7 years ago

FITS2NDF doesn't seem to correctly identify the SMURF extension, so converting reduced SCUBA-2 data to FITS and back (as the JSA does to generate co-adds) I see this kind of HDS structure:

   MORE           <EXT>           {structure}
      FITS(322)      <_CHAR*80>      '...'
      SMURF          <QUALITY_NAMES>   {structure}
         QUALITY_NAMES  <QUALITY_NAMES>   {structure}
            LAST_USED      <_INTEGER>      2

Then because the SMURF extension's type is QUALITY_NAMES (rather than SMURF_EXT), NDF2FITS doesn't recognize it when converting the co-add back to FITS format. It does:

                  SMURF = XTYPE .EQ. 'SMURF' .OR.
     :                    XTYPE .EQ. 'SMURF_EXT'

So the SMURF extension is written out as binary tables rather than via COF_SMURF.

I'm not sure if COF_F2NDF's special handling of SMURF files via COF_SMFIM would help generate the correct type SMURF extension in the first palce -- I didn't look at that in detail, but COF_SPEC attempts to identify SMURF files like this:

      ELSE IF ( TELPRE .AND. TELESC .EQ. 'JCMT' .AND.
     :          INSPRE .AND. INSTRU .EQ. 'HARP'  ) THEN
         NAME = 'SMURF'
      END IF

so it won't identify SCUBA-2 observations as being from SMURF.

In commit cf4943319baa87774aac6bba81ab1f1c831c09d0 (on the 2015A-legacy branch), I added a couple of temporary checks for ways the SMURF extension was being created for the reduced SCUBA-2 FITS files I had to hand, to set the type to SMURF_EXT.

dsberry commented 7 years ago

COF_SPEC is definitely wrong. I've changed it to use Malcolm's own suggestion of looking for FITS extensions that refer to ".MORE.SMURF"

This doesn't solve the whole problem since the correct HDS type for the SMURF extension (i.e. SMURF_EXT) is not recorded in the FITS file anywhere. What does seems to work though is modifying cof_smurf.f so that it always call cof_wstr regardless of the value of NONNDF. This causes an extra dummy FITS extension to be added to the FITS file that does contain the real type "SMURF_TYP" for the extension, and this extension is used by fits2ndf to recreate the correct type. But would including this extra FITS extension have implications for CADC? The headers for it look like this:

XTENSION= 'BINTABLE' / binary table extension BITPIX = 8 / 8-bit bytes NAXIS = 2 / 2-dimensional binary table NAXIS1 = 4 / width of table in bytes NAXIS2 = 1 / number of rows in table PCOUNT = 0 / size of special data area GCOUNT = 1 / one data group (required keyword) TFIELDS = 1 / number of fields in each row TTYPE1 = 'DUMMY_FOR_STRUC' / label for field 1 TFORM1 = '1J ' / data format of field: 4-byte INTEGER TNULL1 = -2147483648 / Starlink bad value EXTNAME = 'OUT.MORE.SMURF' / name of this binary table extension EXTLEVEL= 3 / Level in the hierarchical structure EXTTYPE = 'SMURF_EXT' / HDS data type of the primitive object EXTSHAPE= '0 ' / Shape of the hierarchical structure END

There's an example fits file at /home/dberry/out.fit.gz at Hilo.

MalcolmCurrie commented 7 years ago

I can take a look at this failure to restore the original NDF structure tomorrow when I'm next at RAL.

Malcolm

grahambell commented 7 years ago

This causes an extra dummy FITS extension [...] But would including this extra FITS extension have implications for CADC?

That shouldn't be a problem for CADC. We currently just assume we know what the primary HDU and first extension are for most files in the JSA. (Those typically being "science" and noise respectively.) Even if it were a problem, since we now run all the ingestion software at EAO in Hilo, we could easily make any necessary changes.

dsberry commented 7 years ago

Thanks @MalcolmCurrie - I'll leave it to you then.

MalcolmCurrie commented 7 years ago

COF_SPEC is definitely wrong. I've changed it to use Malcolm's own suggestion of looking for FITS extensions that refer to ".MORE.SMURF"

COF_SPEC was modified for SMURF in early 2008, a few years before SCUBA-2 became a productive instrument. The documentation does say it supports

implying HARP data.

Why I hadn't implemented a COF_ISSMF routine was probably to avoid a search through all the FITS extensions, which might be inefficient dealing with some big HARP cubes.

This doesn't solve the whole problem since the correct HDS type for the SMURF extension (i.e. SMURF_EXT) is not recorded in the FITS file anywhere. What does seems to work though is modifying cof_smurf.f so that it always call cof_wstr regardless of the value of NONNDF. This

The logic is wrong for SCUBA-2 data, which hadn't been written at the time. The number of non-NDFs is not zero because of the QUALITY_NAMES structure array. The code says a non-NDF would create a binary table that would propagate the SMURF structure. That may have been true at the time, but it's not the case now. There may have been a change elsewhere (e.g. COF_THIER) breaking this functionality. Given the extra code to count the non-NDF compoents and this behaviour mentioned in the COF_WSTR comments re-enforcing that this property was once true.

For heterodyne cubes MAKECUBE generates the two or three NDFs in .MORE.SMURF and so works just fine. I located an old HARP reduced cube which also had the QUALITY_NAMES array, and it too failed to make the dummy BINTABLE propagating the SMURF extension name and type. I think David's suggestion to always create this BINTABLE regardless of the number of non-NDF components has merit, and I've modified COF_SMURF accordingly.

It would be reassuring to find what has changed. I thought that at first that it might be the array structure undermining some logic, but the same happens if I add a primitive component to a HARP SMURF extension containing just three NDFs.

I will perform some more tests before pushing a commit.

causes an extra dummy FITS extension to be added to the FITS file that does contain the real type "SMURF_TYP" for the extension, and this extension is used by fits2ndf to recreate the correct type.

The small BINTABLE is what is created for HARP data, and permits the recreation of the extension structure. So it shouldn't be a problem for CADC.

Malcolm