History text lost on conversion to FITS and back causing PROVSHOW to segfault

[grahambell]

ORAC-DR writes SCUBA-2 JSA tiles with history information:

prov-problem-in $ hislist s20140301_00007_850_healpix001244.sdf
...
2: 2014 Apr 29 00:08:40.682 - SETTITLE        (KAPPA 2.1-12)

   Parameters: NDF=@s20140301_00007_850_fmos_1244 TITLE='CRL618'
      MSG_FILTER=! QUIET=!
   Software: /net/ipu/export/data/gbell/star/bin/kappa/ndfpack_mon

And jsawrapdr converts it to FITS including the history:

prov-problem-in $ fitshead jcmts20140301_00007_850_healpix001244_obs_000.fits
...
HISTORY 2: 2014 Apr 29 00:08:40.682 - SETTITLE        (KAPPA 2.1-12)
HISTORY User: gbell   Host: ikaika  Width: 72
HISTORY Dataset: /export/data/visitors/gbell/scratch19/aIN5sFKqj5/s20140301_0000
HISTORY 7_85...
HISTORY Parameters: NDF=@s20140301_00007_850_fmos_1244 TITLE='CRL618'
HISTORY    MSG_FILTER=! QUIET=!
HISTORY Software: /net/ipu/export/data/gbell/star/bin/kappa/ndfpack_mon

Then jsawrapdr converts it back to SDF and the history is missing:

prov-problem-out $ hislist s20140301_00007_850_healpix001238.sdf
...
2: 2014 Apr 28 23:51:05.076 - SETTITLE        (KAPPA 2.1-12)

(and PROVSHOW works on that file).

However when ORAC-DR processes that file, I get a file, also without history:

prov-problem-out $ hislist gs850um_healpix001244.sdf
...
2: 2014 Apr 28 23:51:05.218 - SETTITLE        (KAPPA 2.1-12)

But now PROVSHOW segfaults:

prov-problem-out $ provshow gs850um_healpix001244.sdf
Segmentation fault (core dumped)

(Which prevents jsawrapdr from post-processing that file.)

I was able to put an if statement around the line which I identified with gdb
as causing the segfault, so I have the following, which works:

ndg_provenance.c:
3169          if (hrec->text) {
3170             astMapPut0C( kmrec, TEXT_NAME, hrec->text, NULL );
3171          }
3172          else {
3173             astMapPut0C( kmrec, TEXT_NAME, "", NULL );
3174          }

But I'm not sure whether HistRec.text is supposed to be allowed to be NULL or
not. If it is then I could commit this change, but if it's supposed to be
guaranteed to not be NULL then this isn't the right fix.

[timj]

My inclination is that this is not meant to happen in the sense that a roundtrip is meant to get you back where you started. ndf2fits->fits2ndf is meant to be lossless (and @MalcolmCurrie works hard for that to be true). It still sounds like your patch is a good safety net.

Note that NDF does not start history recording if there is no history structure so to enable history recording ORAC-DR (presumably picard) would have to enable history on the file (and it shouldn't do that on the input file as we probably shouldn't be modifying files given to the pipeline).

[MalcolmCurrie]

Since I have other jobs to complete first this week, I asked Graham to post this as an issue.
Some change has derailed the history propagation back from FITS to NDF. It used to work,
and I certainly want it going again.

[dsberry]

@grahambell Can you send me, or tell me where to find, the gs850um_healpix001244.sdf file that causes provshow to seg fault?

[dsberry]

Commit 9769826 should fix the segfault in provshow. But there is still the issue of why fits2ndf creates blank history text.

[MalcolmCurrie]

This is ironic. FITSIO has changed the way it handles long history headers.
It now writes continuation lines in 70-character blocks. In hindsight it would
have been better to have dealt with this in COF_WHISR (now CVG_WHISR),
making my own paragraphs for all not just some of the history components
like Parameters, Arguments, and Group. We're falling foul of long names
for the Dataset, Software, and Command elements.

I propose to modify CVG_WHISR to make paragraghs for these elements
rather than leave it to FITSIO, which might change, This won't affect old
NDF2FITS-created HISTORY, because it's truncated. This change has
the advantage of restoring all the information. At present the cycle can be
lossy.

I can then make COF_CHISR recognise the continuation lines for these
additional elements and recover the full names for those of you who like
very long paths.

[MalcolmCurrie]

A second factor was the buffer size for the text records. The SMURF
configuration list and group parameters when running MAKEMAP can
required several thousand characters. For now I've upped the buffer
size, but some dynamic allocation would be better.

[MalcolmCurrie]

Partial fixes are in af56e5a and 4acde70 (not the original 32f0666f8ed08
as stated in a commit). There is a change needed to NDF2FITS too, so not closing
this issue yet.

[MalcolmCurrie]

I've made a change to NDF2FITS too (0b4b02c).
@grahambell please reprocess from the NDF (not the FITS) to check that the round trips
are now correct again.

[timj]

So can this ticket be closed?

[MalcolmCurrie]

I just wanted confirmation from Graham, but I believe it can be closed. The history
records are not quite identical in terms of indentation, but should have the same
content.

[grahambell]

David's fix prevented the segfault I was encountering, so the main issue is fixed. The history looked mostly OK as far as I remember -- I'm not sure if the colons in Perl module names were still being a problem or not.