CDF fixes

[egthomas]

This pull request is a partial attempt at addressing the issues raised in #211 by adding the --version option to several additional binaries, cleaning up spacing in the dmaptocdf code, and fixing a bug present in both dmaptocdf and dmaptoncdf. The original problem with handling ltab arrays still remains to be fixed.

[egthomas]

I just tested the netCDF conversion tool and dmaptoncdf was entirely unable to write int format values to the output file. My attempt at fixing it appears to have introduced another problem, where sometimes a -60 error code is returned by nc_put_var1_long midway through the dmaptoncdf routine (the lines beginning with sx->name are debugging statements I added):

dmaptoncdf 20181129.cve.fitacf fitacf.cdfmap 20181129.cve.netcdf
...
sx->name: mxpwr, sx->type: 3, sy->name: mxpwr, sy->type: 3
sx->name: mxpwr, sx->data.iptr: 1073741824
Error writing CDF file (-60).

[asreimer]

I'm getting a -2005 error code:

$ dmaptoskeleton 20170505.C0.sas.fitacf fitacf.skt fitacf.cdfmap
$ skeletoncdf -cdf 20170505.C0.sas.cdf fitacf.skt
Making a cdf from skeleton table: "fitacf.skt"  to  "20170505.C0.sas.cdf"...
$ dmaptocdf 20170505.C0.sas.fitacf fitacf.cdfmap 20170505.C0.sas.cdf 
Error writing CDF file (-2005).

Same thing for another fitacf file, which I've attached since it is smaller:
$ dmaptoskeleton 20180624.0601.00.hkw.fitacf fitacf.skt fitacf.cdfmap
$ skeletoncdf -cdf 20180624.0601.00.hkw.cdf fitacf.skt
Making a cdf from skeleton table: "fitacf.skt" to "20180624.0601.00.hkw.cdf"...
$ dmaptocdf 20180624.0601.00.hkw.fitacf fitacf.cdfmap 20180624.0601.00.hkw.cdf
Error writing CDF file (-2005).

20180624.0601.00.hkw.fitacf.gz

Did I just do something stupid?

I'm running out of time for debugging today, so I'm just reporting what I have. By the way, the NCAR link posted above is unreachable for me. NASA has documentation here: https://cdf.gsfc.nasa.gov/html/cdf_docs.html

The C reference manual is probably helpful. Looking at Chapter 8, it looks like we can make debugging easier by printing the CDF status message when there's an error.

Finally, the scope of this PR is fixing CDF problems, but I did check which binaries the --version flag works with. Including list in case it's useful:

addtext, makeall, make_sim, mkidlcoef, padpng, padppm, padppmx, padxml, pngd, pngtoppm, pngtoppmx, pngtoxml, ppmd, ppmtopng, ppmtoppmx, ppmtoxml, ppmxd, ppmxtopng, ppmxtoppm, ppmxtoxml, scalepng, scaleppm, scaleppmx, scalexml, scdoc, shmemrecv, shmemsend, sim_real, test_aacgm, test_fitacf, test_fitex2, test_lmfit, xmld, xmldoc, xmltopng, xmltoppm, xmltoppmx

[egthomas]

As noted in #211 and in the description of this pull request, I haven't been able to sort out the BAD_DIM_INDEX (-2005) / BAD_DIM_COUNT (-2039) error codes encountered with dmaptocdf.

[asreimer]

Good to know. I didn't see -2005 referenced anywhere, so I didn't know it was the same BAD_DIM_INDEX that you were referring too.

[ksterne]

So, @egthomas, any thoughts on why the -60 error code is now popping up? Is this a bug with the rst code or an error with the test file you're using?

Can you describe a little more on the bug that you did fix in the dmaptocdf and dmaptonetcdf binaries? More specifically, what's the test that shows develop has a bug that this PR fixes?

[ksterne]

@egthomas, bump here on thoughts at why the -60 error code is now happening?

[mts299]

What is the status on this PR?

[ksterne]

I'm thinking this PR has a bit of scope creep to it getting into the error codes that we're seeing. @egthomas, do you remember the original bug that you fixed with this code? Can we test that at least that bug is fixed and save looking into the error codes another day?

[egthomas]

I'm thinking this PR has a bit of scope creep to it getting into the error codes that we're seeing.

Fixing those error codes was the original goal of #211 and this pull request, but after more than a year it has still not been accomplished.

@egthomas, do you remember the original bug that you fixed with this code?

As stated in the previous messages, these are the changes made by this pull request:

1. The --version option was added to the remaining binaries in the general directory. It appears @asreimer tested this functionality on June 21 last year.
2. The spacing in dmaptocdf, dmaptoskeleton, and dmaptoncdf was cleaned up for easier readibility.
3. Fixed the same bug in the loadmap functions of dmaptocdf.c and dmaptoncdf.c which caused incorrect memory allocation when dealing with arrays in dmap-format files.
4. Fixed another bug in loadmap of dmaptoncdf.c where int-type scalars were not being correctly identified when loading a dmap-format file which ultimately prevented them from being written to the output netcdf file. It's possible this change may also need to be implemented: https://github.com/egthomas/rst/commit/551c7d500fe05f95fa7d519ce1dcf29a7a5746c3

To summarize, the dmaptocdf and dmaptoncdf routines still don't work after this pull request, but they didn't work before it either. This pull request implemented several bug fixes to make progress toward returning the functionality of those conversion routines. In my opinion, this is still worth merging even though the original issue of handling two- (or multi-) dimensional arrays (eg, ltab) identified in #211 more than a year ago has not yet been solved.

[ecbland]

the dmaptocdf and dmaptoncdf routines still don't work after this pull request, but they didn't work before it either. This pull request implemented several bug fixes to make progress toward returning the functionality of those conversion routines.

@egthomas Is there anything we can do to test the particular bug fixes in this PR, or do you recommend that we just merge it without any further testing? I realise testing might be difficult since the dmaptocdf and dmaptoncdf routines still don't work.

[egthomas]

Unfortunately I can't think of any tests for dmaptocdf. I went back to try dmaptoncdf and the situation is not quite as bad as I had thought.

For dmaptoncdf, the time_us, intt_us, mxpwr, lvmax, fitacf_revision_major, and fitacf_revision_minor fields in the output netcdf file are all empty when using the develop branch, but will be correctly populated when using this branch after the latest commit. This also addresses the -60 error code previously encountered after the initial attempt to fix dmaptoncdf.

Note that because the radar_revision_major, radar_revision_minor, origin_code, qflg, and gflg fields (among others) are all of type char but are used to store integer-style values, they will often not be correctly written to the output netcdf format file. I considered converting all char fields to short in the conversion routine, but then remembered that there are several other text-based fields of type char which need to remain as is (eg origin_command, combf, etc).

[ecbland]

Any objections to merging this PR? @ksterne @asreimer

[asreimer]

To fully fix CDF stuff, open another PR. It's going to take someone sitting down and going through the CDF API in detail.