Opened 6 years ago

Closed 5 years ago

#4294 closed enhancement (fixed)

changes to the netcdf driver export and import

Reported by: etourigny Owned by: etourigny
Priority: normal Milestone:
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords: netcdf
Cc: pds

Description

This ticket serves as a central point to comment on proposed changes to the export and import of the netcdf driver and posting patches and small test filess. The following wiki pages describe the proposed changes and improvements:

A more complete description of all planned changes to the driver can be viewed at

Current code resides in a Git repository at https://github.com/etiennesky/gdal-netcdf/ within the "gdal-netcdf-proj" branch.

The only files which differ from svn trunk are frmts/netcdfdataset.{h,cpp} and they will be posted here as attachments for convenience. Anyone that wishes to contribute can post patches here or given write access to the repository.

Attachments (3)

netcdfdataset.h (20.7 KB) - added by etourigny 6 years ago.
header file as of oct. 18
netcdfdataset.cpp (171.8 KB) - added by etourigny 6 years ago.
source file as of oct. 18
tmp3-2.nc (17.6 KB) - added by etourigny 5 years ago.

Download all attachments as: .zip

Change History (19)

Changed 6 years ago by etourigny

Attachment: netcdfdataset.h added

header file as of oct. 18

Changed 6 years ago by etourigny

Attachment: netcdfdataset.cpp added

source file as of oct. 18

comment:1 Changed 6 years ago by etourigny

the branch on github has been renamed to 'netcdf-dev'

comment:2 Changed 6 years ago by etourigny

comments on latest commits:

Various bugs fixed / new features:

- bottom-up by default #4284, with config option GDAL_NETCDF_BOTTOMUP to override import and export
  and create option WRITE_BOTTOMUP
- fix projected export+import CF
- added simple progress
- metadata export fixes + add_offset/scale_factor (bug #4211)
- added missing	case NC_SHORT: in netCDFRasterBand::CreateBandMetadata( ) 


Bugs that need fixing:

- support import of unequally-spaced grids
- support of lon/lat grids from CF grids which are not supported
- support variable with multiple values in metadataitem (CreateBandMetadata), 
  this should use a generic function which detects the var type also.

TODO

- test signed byte import
// Detect unsigned Byte data 
    int bUnsignedByte = TRUE;
if ( this->eDataType == GDT_Byte &&
( atttype == NC_CHAR && EQUAL(szMetaTemp,"_Unsigned") 
&& ( EQUAL(szTemp,"true") || EQUAL(szTemp,"1") ) ) ) {
bUnsigned = TRUE;
printf("Is unsigned\n");
}

comment:3 Changed 6 years ago by Even Rouault

Etienne,

I get a compilation error with netCDF 3.6.3. Tamas' bots too.

netcdfdataset.cpp: In constructor ‘netCDFRasterBand::netCDFRasterBand(netCDFDataset*, int, int, int, int*, int*, int)’:
netcdfdataset.cpp:509: error: ‘NC_UBYTE’ was not declared in this scope
netcdfdataset.cpp: In function ‘GDALDataset* NCDFCreateCopy(const char*, GDALDataset*, int, char**, int (*)(double, const char*, void*), void*)’:
netcdfdataset.cpp:3717: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3717: error:   initializing argument 3 of ‘int nc_def_var(int, const char*, nc_type, int, const int*, int*)’
netcdfdataset.cpp:3722: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3722: error:   initializing argument 4 of ‘int nc_put_att_schar(int, int, const char*, nc_type, size_t, const signed char*)’
netcdfdataset.cpp:3769: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3769: error:   initializing argument 3 of ‘int nc_def_var(int, const char*, nc_type, int, const int*, int*)’
netcdfdataset.cpp:3773: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3773: error:   initializing argument 4 of ‘int nc_put_att_short(int, int, const char*, nc_type, size_t, const short int*)’
netcdfdataset.cpp:3817: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3817: error:   initializing argument 3 of ‘int nc_def_var(int, const char*, nc_type, int, const int*, int*)’
netcdfdataset.cpp:3821: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3821: error:   initializing argument 4 of ‘int nc_put_att_int(int, int, const char*, nc_type, size_t, const int*)’
netcdfdataset.cpp:3864: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3864: error:   initializing argument 3 of ‘int nc_def_var(int, const char*, nc_type, int, const int*, int*)’
netcdfdataset.cpp:3868: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3868: error:   initializing argument 4 of ‘int nc_put_att_float(int, int, const char*, nc_type, size_t, const float*)’
netcdfdataset.cpp:3911: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3911: error:   initializing argument 3 of ‘int nc_def_var(int, const char*, nc_type, int, const int*, int*)’
netcdfdataset.cpp:3914: error: invalid conversion from ‘int’ to ‘nc_type’
netcdfdataset.cpp:3914: error:   initializing argument 4 of ‘int nc_put_att_double(int, int, const char*, nc_type, size_t, const double*)’

comment:4 Changed 6 years ago by etourigny

Fixed in r23269. Thanks for the alert. It's a netcdf3 vs. netcdf4 issue, tested with netcdf-3 and runs ok.

I really have to find a way to detect the netcdf capabilities (such as nc4, libz, etc) in configure. Do you know how I can test for libz support at run-time?

I haven't finished all the commits though, because I ran into a last-minute problem and didn't have time to finalize yesterday.

comment:5 Changed 6 years ago by Even Rouault

Etienne, I think you have forgotten to commit an updated configure.in in r23272

comment:6 Changed 6 years ago by etourigny

Added NC4, NC4C and NC2 formats for export (issue #3166), as well as HDF5 and HDF4 import, and updated configure, GDALmake.opt.in , and netcdf GNUMakefile (r23272).

Updated configure.in and configure (r23274).

To ensure proper support of import and export, please re-configure and re-compile netcdfdataset. Only works in linux/unix for now, as Win32 config has not been updated.

The driver now supports reading HDF4 and HDF5 files with the syntax "NETCDF:file.hdf". Without this syntax, proper drivers are used if supported by GDAL installation, and if not the netcdf driver is used.

comment:7 Changed 6 years ago by Even Rouault

r23276 /trunk/gdal/ (frmts/netcdf/makefile.vc nmake.opt): netCDF: windows build : add support NETCDF_HAS_NC4, NETCDF_HAS_NC5, HAVE_HDF4, HAVE_HDF5 (#4294)

comment:8 Changed 5 years ago by Even Rouault

Etienne,

I've a problem with the reordering in configure.in done in r22274. With the new order, I can no longer open any HDF4 files.

This is due to the relative ordering of -lnetcdf with -lmfhdf -ldf. Since r22274, we have LIBS = -lnetcdf [stiff] -lmfhdf -ldf (in GDALmake.opt). Whereas before it was LIBS = -lmfhdf -ldf [stuff] -lnetcdf

If I put again -lnetcdf after -lmfhdf -ldf, it works. I think the reason is that libmfhdf and libnetcdf export identical symbols ...

objdump -T /usr/lib/libnetcdf.so | grep Base | awk '{print $7}' | sort > netcdf.txt
objdump -T /usr/lib/libmfhdf.so | grep Base | awk '{print $7}' | sort > mfhdf.txt
for i in `cat netcdf.txt`; do grep $i mfhdf.txt ;done

My versions of HDF4 and netCDF are (Ubuntu 10.04 64bit) :

ii  libnetcdf-dev                        1:3.6.3-1                                       Development kit for NetCDF
ii  libnetcdf4                           1:3.6.3-1                                       An interface for scientific data access to l
ii  libhdf4-0                            4.2r4-10                                        The Hierarchical Data Format library -- libr
ii  libhdf4-0-alt                        4.2r4-10                                        The Hierarchical Data Format library -- libr
ii  libhdf4-dev                          4.2r4-10                                        The Hierarchical Data Format library -- deve

I've commited a "fix" for that issue that makes hdf4 tests work again and doesn't seem to cause issues with netcdf tests on my system. But you'd better test with your system and netcdf4...

r23322 /trunk/gdal/ (configure configure.in): configure: make sure that -lnetcdf appears after -lmfhdf -ldf on linking line. Ugly... (#4294)

comment:9 Changed 5 years ago by etourigny

Even,

The fix is ok at first glance (it doesn't cause conflicts with my working install), although it addresses the symptom rather that the cause. It might cause unexpected behavior to have conflicting libs (as I will show below). I would prefer to revert the 1 liner so that it would be like the other libs. Proper fix is below (replacing libhdf4-dev with libhdf4-alt-dev).

I think the fundamental problem is the way that hdf4 is configured in your system, because hdf4 was configured with netcdf support and conflicts with libnetcdf, as described in wiki:HDF .

From configure.in:

dnl Debian supplies the HDF4 library which does not conflict with NetCDF.
dnl Test for Debian flavor first. Hint: install the libhdf4-alt-dev package.
  AC_CHECK_LIB(mfhdfalt,SDreaddata,HDF_LIB_NAME="-lmfhdfalt -ldfalt",HDF_LIB_NAME=missing,-ldfalt)

dnl If it fails, test again for normal libmfhdf/libdf

You probably need to install libhdf4-alt-dev and remove libhdf4-dev. From there GDAL configure should detect mfhdfalt and use it instead of mfhdf.

I added 2 autotests in r23350 to test hdf4 support and conflict with netcdf driver.

I managed to reproduce your problem by removing libmfhdfalt from my system and hand installing a conflicting hdf4 install (with netcdf) and netcdf-3 (without hdf4 support). In this case test netcdf_22 skipped (no hdf4 support in netcdf driver) and netcdf_23 failed (no hdf4 support at all).

However, by changing the order of -lnetcdf and -lmfhdf (in all places in configure where -lnetcdf is added) resulted in a segfault in the netcdf driver:

(gdb) file gdalinfo 
Reading symbols from /home/softdev/bin/gdalinfo...done.
(gdb) run  NETCDF:data/trmm.nc
Starting program: /home/softdev/bin/gdalinfo NETCDF:data/trmm.nc
[Thread debugging using libthread_db enabled]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff515a850 in NC_var_shape () from /home/softdev/lib/libmfhdf.so.0
(gdb) bt
#0  0x00007ffff515a850 in NC_var_shape () from /home/softdev/lib/libmfhdf.so.0
#1  0x00007ffff3928f7d in NC_computeshapes (ncp=0x625a20) at v1hpg.c:1112
#2  nc_get_NC (ncp=0x625a20) at v1hpg.c:1395
#3  0x00007ffff3926e10 in nc__open_mp (path=0x625a08 "data/trmm.nc", ioflags=0, basepe=0, chunksizehintp=0x0, ncid_ptr=0x7fffffff35a8) at nc.c:1098
#4  0x00007ffff734c87f in netCDFDataset::Open (poOpenInfo=0x7fffffff3870) at netcdfdataset.cpp:2569
#5  0x00007ffff744e208 in GDALOpenInternal (oOpenInfo=..., papszAllowedDrivers=0x0) at gdaldataset.cpp:2236
#6  0x00007ffff744e3a2 in GDALOpenInternal (pszFilename=<value optimized out>, eAccess=<value optimized out>, papszAllowedDrivers=0x0) at gdaldataset.cpp:2208
#7  0x0000000000402985 in main (argc=2, argv=0x624e40) at gdalinfo.c:173

In summary, please try installing libhdf4-alt-dev and removing libhdf4-dev, revert the following diff and try the netcdf.py autotests again.

20988	20992	  if test "$NETCDF_SETTING" = "yes" ; then 
20989	 	    LIBS="-lnetcdf $LIBS" 
 	20993	    LIBS="$LIBS -lnetcdf" 

Thanks

comment:10 Changed 5 years ago by Even Rouault

I followed your advice and it worked as you described. So : r23351 /trunk/gdal/ (configure configure.in): HDF4 configure: revert r23322 : The proper solution is to install libhdf4-alt-dev package on Debian/Ubuntu? systems (#4294)

comment:11 Changed 5 years ago by etourigny

commit to trunk (r23454):

  • improved metadata import and export (esp. array attributes which was buggy for Band data) and simplified code with new functions NCDFGetAttr and NCDFPutAttr
  • added autotests to check for better support for nodata (with BYTE data)

comment:12 Changed 5 years ago by Even Rouault

r23455 fixes the following compilation problem with MSVC :

netcdfdataset.cpp(707) : error C2668: 'fabs' : ambiguous call to overloaded function
        E:\vs90\VC\INCLUDE\math.h(557): could be 'long double fabs(long double)'
        E:\vs90\VC\INCLUDE\math.h(509): or       'float fabs(float)'
        E:\vs90\VC\INCLUDE\math.h(119): or       'double fabs(double)'
        while trying to match the argument list '(int)'
        netcdfdataset.cpp(806) : see reference to function template instantiation 'void netCDFRasterBand::CheckValidData<signed char>(void *,int)' being compiled

comment:13 Changed 5 years ago by etourigny

the fix does not work with Byte data (does not respect valid_range, which sets min amd max).

Attaching an example file that does not work with the fix:

$ gdalinfo -stats -checksum tmp3-2.nc [...]

Metadata:

_FillValue=0 grid_mapping=transverse_mercator long_name=GDAL Band Number 1 NETCDF_VARNAME=Band1 STATISTICS_MAXIMUM=231 STATISTICS_MEAN=73.499388503873 STATISTICS_MINIMUM=10 STATISTICS_STDDEV=23.290183192411 valid_range={ 20, 30 }

Whereas output should be (min=20, max=30):

Metadata:

_FillValue=0 grid_mapping=transverse_mercator long_name=GDAL Band Number 1 NETCDF_VARNAME=Band1 STATISTICS_MAXIMUM=30 STATISTICS_MEAN=24.875 STATISTICS_MINIMUM=20 STATISTICS_STDDEV=3.3372372971933 valid_range={ 20, 30 }

Changed 5 years ago by etourigny

Attachment: tmp3-2.nc added

comment:14 Changed 5 years ago by etourigny

my mistake, fix works

comment:15 Changed 5 years ago by etourigny

r23616 : only write grid_mapping attributes when bWriteGridMapping == TRUE (#2129, #2893 and #4294)

The problem was that files without grid_mapping had the spheroid parameters written as global metadata, which is incorrect. Also skip writing any projection info if ( !bIsProjected && !bWriteLonLat ).

comment:16 Changed 5 years ago by etourigny

Resolution: fixed
Status: newclosed

closing this ticket as it seems stable

Note: See TracTickets for help on using tickets.