Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#4284 closed defect (fixed)

Change netcdf driver to read and write in the bottom-up order by default

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

Description

This is a spin off ticket: #2893 and transferred from ticket: #4251

Currently the driver (GDAL) expects top-down for projected crs but netCDF is bottom-up approach.

The driver needs to read and write bottom-up by default, unless it is detected that the file was written by and older version of the netcdf driver (which wrote top-down). Will soon be released along with other changes in the driver.

This fixes issues where there crs is not recognized (i.e. rotated pole, bug #4251)

A quick fix to fix proper (bottom-up) import is to set bBottomUp = TRUE in the netCDFDataset constructor, however this may cause problems with files created by the old (current) netcdf driver.

And to fix the proper export, around line 369 (in the part that deals with projected crs):

 /* netcdf standard is bottom-up, but leave it top first for now */
       bBottomUp = TRUE;

Change History (9)

comment:1 Changed 5 years ago by etourigny

Keywords: netcdf added

The change to default bBottom=TRUE for import causes autotests netcdf_1, netcdf_4 and netcdf_5 to fail, because the driver reads them as bottom-up instead of top down, which changes the checksum for the band.

A better fix is the following (in SetProjection?()):

 status = nc_get_vara_double( cdfid, nVarDimYID, 
                                     start, edge, pdfYCoord);
+        poDS->bBottomUp = TRUE;
+        if ( pdfYCoord[0] > pdfYCoord[1] )
+            poDS->bBottomUp = FALSE;

The downside is that if there is no geo information, it *might* be upside down, but this is less likely.

With this change autotests netcdf_4 and netcdf_5 still fail (there is no geo info in those files), but removing the checksum test fixes that.

comment:2 Changed 5 years ago by peifer

Would it make sense to have some config option (NETCDF_BOTTOMUP or similar) that could be used in cases when the driver would misinterpret the order?

comment:3 Changed 5 years ago by etourigny

Are you suggesting this for import or for export?

For export, I have added the BOTTOM_UP=YES/no creation flag, but not sure how to do this for import, maybe an environment variable, like GDAL_NETCDF_BOTTOMUP? It would be unmanageable to add this as a commandline variable.

comment:4 Changed 5 years ago by peifer

I meant for import. Command line switch or environment variable: I wouldn't care, as long as I can avoid changing the source code for getting around a potential issue.

comment:5 Changed 5 years ago by etourigny

Ok makes sense, I'll add both a -co option for export (should anyone prefer the previous top-down export), and also a Config Option, which can be set either through an environment variable or command line (--config OPTION) as described in wiki:ConfigOptions.

Should the release of the new driver take much time, I'll add a fix to trunk.

comment:6 Changed 5 years ago by etourigny

Resolution: fixed
Status: newclosed

Default behavior in trunk (as of r23459) is bottom-up for import and top-down for export. Netcdf files are expected to be bottom-up, unless the dimension info states otherwise. Therefore import uses bottom-up, unless the file was writen by GDAL, or dimension states the file is top-down. Export is top-down by default (unless there is no dimension info to export), as other software can deal with top-down files as long as there is dimension info. Simple becnhmarks have shown no performance hit.

driver option for creation: WRITE_BOTTOMUP=YES/NO (default: NO)

Config Options GDAL_NETCDF_BOTTOMUP=YES/NO (default:NO) sets the default of WRITE_BOTTOMUP

and also the default behavior for import

comment:7 Changed 5 years ago by etourigny

Revised version: now default is top-down for export and bottom-up for import (to catch files without geo-referencing information)

comment:8 Changed 5 years ago by etourigny

r23615: fix upside-down export and import of grids without projection and geotransform (#2129, #4284)

There was a problem with files lacking georeferencing (or grids not supported by the driver), they were written in the default top-down which confused other apps (such as ncview and even QGis. Now whenever file does not have a geotransform it is written(and read) in the netcdf-style bottom-up.

comment:9 Changed 5 years ago by etourigny

r23617 : partial revert of r23615 and set WRITE_BOTTOMUP default to YES for export and import of files without projection and GT (#2129, #4284)

previous fix broke many autotests, solution is to set bottom-up to TRUE by default, hen projection and GT are detected then the proper order is set (import and export). Hopefully this resolves a thorny question!

Note: See TracTickets for help on using tickets.