Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-24 Thread Glynn Clements

Markus Metz wrote:

> >> Considering this, rather use e.g. raster_min - 1 as default nodata value 
> >> for GDAL floating point datatypes?
> >
> > If fabs(raster_min) is large, raster_min - 1 won't be exactly
> > representable, and may be rounded to raster_min. You would need to
> > subtract a value which depends upon the exponent.
> >
> > Also, the input data may contain -DBL_MAX. For FCELL, if the map is
> > large enough, it's possible that *all* finite values occur in the
> > data.
> I was (honestly! I was listening to you:-)) thinking about something like
> 
> if (raster_min > type_min) {
> unsigned int i = 0;
> while ((float)(raster_min - ++i) == raster_min);

If fabs(raster_min) is large enough, even subtracting 2^32 may not be
enough to produce a distinct value. Try:

double i = 1
while ((float)(raster_min - i) == (float)raster_min)
i *= 2;

Except, I'm not sure that a simple cast to float is sufficient. In the
absence of -ffloat-store, the values and the comparions may be 80-bit.

I would suggest using frexp() and ldexp(), e.g.:

int exp;

frexp(raster_min, &exp);
nodataval = raster_min - ldexp(1.0, exp - 23);

> > The question is: is GDAL's no-data value allowed to be any FP value,
> > or must it be finite? I'm guessing that the GDAL documentation doesn't
> > say (similarly, I don't think it's documented whether GRASS
> > FCELL/DCELL values are allowed to be infinite).
> >
> > Actually, this may depend upon the format. If a format supports FP,
> > the format's specification may dictate whether or not NaN and/or
> > inifinities are considered valid.
> 
> Hmm. I thought the point of using GDAL is that r.out.gdal does *not* 
> have to bother about the format's specifications. IMHO, if there is 
> reason to assume that NaN and/or infinities are not valid for any of the 
> formats supporting FP, r.out.gdal should not use NaN and/or infinities. 
> OTOH, NaN and infinities are described in the IEEE FP standard. I would 
> avoid format-specific code in r.out.gdal, considering the large number 
> of supported formats and the number of creation options for each format 
> (for each format sometimes differing between datatypes, e.g. GeoTIFF).

By that reasoning, r.out.gdal shouldn't write floats, as many of the
formats only support integers.

If you don't use NaN, what will you do when presented with data which
contains both -FLT_MAX and FLT_MAX (or DBL_*)?

Also, NaN has the advantage that software will tend to treat it as
"null" by default (it isn't coincidence that GRASS' nulls behave like
NaN).

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-23 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:

Considering this, rather use e.g. raster_min - 1 as default nodata value 
for GDAL floating point datatypes?


If fabs(raster_min) is large, raster_min - 1 won't be exactly
representable, and may be rounded to raster_min. You would need to
subtract a value which depends upon the exponent.

Also, the input data may contain -DBL_MAX. For FCELL, if the map is
large enough, it's possible that *all* finite values occur in the
data.

I was (honestly! I was listening to you:-)) thinking about something like

if (raster_min > type_min) {
unsigned int i = 0;
while ((float)(raster_min - ++i) == raster_min);
nodata = raster_min - i;
}
else if (raster_max < type_max) {
unsigned int i = 0;
while ((float)(raster_max + ++i) == raster_max);
nodata = raster_max + i;
}
else
nodata = 0; /* or whatever fallback */

for GDT_Float32 but did not want to go into so much detail.


The question is: is GDAL's no-data value allowed to be any FP value,
or must it be finite? I'm guessing that the GDAL documentation doesn't
say (similarly, I don't think it's documented whether GRASS
FCELL/DCELL values are allowed to be infinite).

Actually, this may depend upon the format. If a format supports FP,
the format's specification may dictate whether or not NaN and/or
inifinities are considered valid.
Hmm. I thought the point of using GDAL is that r.out.gdal does *not* 
have to bother about the format's specifications. IMHO, if there is 
reason to assume that NaN and/or infinities are not valid for any of the 
formats supporting FP, r.out.gdal should not use NaN and/or infinities. 
OTOH, NaN and infinities are described in the IEEE FP standard. I would 
avoid format-specific code in r.out.gdal, considering the large number 
of supported formats and the number of creation options for each format 
(for each format sometimes differing between datatypes, e.g. GeoTIFF).


___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-23 Thread Glynn Clements

Markus Metz wrote:

> Considering this, rather use e.g. raster_min - 1 as default nodata value 
> for GDAL floating point datatypes?

If fabs(raster_min) is large, raster_min - 1 won't be exactly
representable, and may be rounded to raster_min. You would need to
subtract a value which depends upon the exponent.

Also, the input data may contain -DBL_MAX. For FCELL, if the map is
large enough, it's possible that *all* finite values occur in the
data.

The question is: is GDAL's no-data value allowed to be any FP value,
or must it be finite? I'm guessing that the GDAL documentation doesn't
say (similarly, I don't think it's documented whether GRASS
FCELL/DCELL values are allowed to be infinite).

Actually, this may depend upon the format. If a format supports FP,
the format's specification may dictate whether or not NaN and/or
inifinities are considered valid.

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-23 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:

  

   - all floating point: IEEE's NaN
  
Problem with NaN? According to IEEE 754, x == y is always FALSE if 
either x or y or both are NaN. Assuming (nodata = 
GDALGetRasterNoDataValue()) == NaN,
Sorry, should have been nodata = GDALGetRasterNoDataValue() and 
isnan(nodata)
 then going through all cells if 
(cell == nodata) will always be FALSE, nodata cells may not be detected? 
Depends on each implementation of nodata detection? Can be solved for 
GRASS, but how is this done in all other GIS applications?



if (x == nodata || isnan(nodata) && isnan(x))

isnan() is C99 and POSIX, or it can be defined as e.g.:

#define ISNAN(x) ((x) != (x))
As previously in ticket #73, I created a MASK and then exported 
elevation from nc_spm_08 as FLOAT32, once with nodata=0 and once with 
default nodata value NaN for GDT_Float32.

GRASS 7 r.in.gdal: all ok
GRASS 7 r.external: all ok

Other GIS applications:
QGIS 1.0.0: all ok
gvSIG 1.1.2: doesn't support the nodata concept, and NaN is displayed 
like 0.0
SAGA 2.0.3: display ok for NaN, but cell query gives 0.0 instead of NaN 
or nodata, and NoData info says 0
ESRI ArcMap 9.2: display ok for nodata=0, not ok when using NaN, NaN 
displayed like value 0.0, and cell query gives NaN instead of nodata


Considering this, rather use e.g. raster_min - 1 as default nodata value 
for GDAL floating point datatypes?

___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-22 Thread Glynn Clements

Markus Metz wrote:

> >- all floating point: IEEE's NaN
> 
> Problem with NaN? According to IEEE 754, x == y is always FALSE if 
> either x or y or both are NaN. Assuming (nodata = 
> GDALGetRasterNoDataValue()) == NaN, then going through all cells if 
> (cell == nodata) will always be FALSE, nodata cells may not be detected? 
> Depends on each implementation of nodata detection? Can be solved for 
> GRASS, but how is this done in all other GIS applications?

if (x == nodata || isnan(nodata) && isnan(x))

isnan() is C99 and POSIX, or it can be defined as e.g.:

#define ISNAN(x) ((x) != (x))

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-22 Thread Moritz Lennert

On 22/04/09 09:51, Markus Metz wrote:
Anyway, now I will stop defending a feature that I was never convinced 
of (allowing this nodata mismatch). Can we now put together a next 
version of r.out.gdal based on this discussion that always interprets 
any user-given nodata value (with warning where appropriate) to make 
sure metadata info and raster data are the same?


+1

Moritz
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-22 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:

  

so the user will have to understand the conversion issues even if they
never use an out-of-range value. Ultimately, its either usability or
flexibility.
  
Without the -f flag I would opt for usability, with the -f flag for 
flexibility.



Having -f affect the interpretation of nodata= also harms usability.
  
I know. There was discussion about that, and Dylan asked for the 
possibility to
"2. include a 'force' option to make r.out.gdal do exactly what your 
command line instructions suggest-- possibly leading to corruption of 
NULL data"
Maybe this referred only to the problem of input data having the 
requested nodata value, and not to allow a mismatch between metadata 
nodata and the value replacing NULL cells?
  
The GDAL API documentation says "To clear the nodata value, just set it 
with an "out of range" value." This is currently possible with the -f 
flag, but r.out.gdal then does not give any information on what value is 
assigned to NULL cells, only what nodata value is written to the metadata.


Right. There's only one nodata= option, and it's performing a dual
function: what to store for null cells and what to set GDAL's no-data
value to. If you want to allow a mismatch, you really need two
options.
  
I thought in this case (allow mismatch) it would be enough to write it 
as double to metadata without prior cast to the selected GDAL datatype.



Yeah, but what are you going to write for nulls in the input data?

Suppose the user specifies a CELL input map (which contains at least
0-255 and null), byte (GDT_UInt8) output, -f, and nodata=. So you
use .0 as GDAL's no-data value, but how will you treat null values
in the input data?
  
In this case, the current code of all C versions of r.out.gdal writes 
(CELL)  for NULLs. This is then cast by the gdal library to GDT_Byte 
(255).
Maybe I misunderstood the requested behaviour for the -f flag, so let's 
not allow a nodata mismatch, convert the nodata value first to the 
selected GDAL datatype if necessary and then we get the same value for 
nodata metadata and raster data where NULLs were replaced.
In this case, there should be a warning because (GDT_Byte) .0 = 255 
is present in the input data and the user is asked to select a different 
nodata value. Export would proceed because of -f.


Anyway, now I will stop defending a feature that I was never convinced 
of (allowing this nodata mismatch). Can we now put together a next 
version of r.out.gdal based on this discussion that always interprets 
any user-given nodata value (with warning where appropriate) to make 
sure metadata info and raster data are the same?


___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-21 Thread Glynn Clements

Markus Metz wrote:

> > so the user will have to understand the conversion issues even if they
> > never use an out-of-range value. Ultimately, its either usability or
> > flexibility.
> 
> Without the -f flag I would opt for usability, with the -f flag for 
> flexibility.

Having -f affect the interpretation of nodata= also harms usability.

> >> The GDAL API documentation says "To clear the nodata value, just set it 
> >> with an "out of range" value." This is currently possible with the -f 
> >> flag, but r.out.gdal then does not give any information on what value is 
> >> assigned to NULL cells, only what nodata value is written to the metadata.
> >
> > Right. There's only one nodata= option, and it's performing a dual
> > function: what to store for null cells and what to set GDAL's no-data
> > value to. If you want to allow a mismatch, you really need two
> > options.
> 
> I thought in this case (allow mismatch) it would be enough to write it 
> as double to metadata without prior cast to the selected GDAL datatype.

Yeah, but what are you going to write for nulls in the input data?

Suppose the user specifies a CELL input map (which contains at least
0-255 and null), byte (GDT_UInt8) output, -f, and nodata=. So you
use .0 as GDAL's no-data value, but how will you treat null values
in the input data?

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-21 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:
  
I think I slowly begin to understand. You suggest to use 
(GDT_Int32)0x8000 for both the GDAL metadata info and the GDAL 
raster data although GDAL metadata nodata is double?



Yes; for the latter, the value would be:

(double) (GDT_Int32) 0x8000

IOW, -2147483648.0.

Note that this isn't the same as atof("0x8000"), which would be
positive.
  
Obviously. Because of the request for a -f flag I was fixed on leaving 
r.out.gdal parameters as they are, also leaving default nodata as 
double, no prior cast to the GDAL datatype. My confusion, it's clear now 
what you want for the default nodata value.
Further on, I understand you suggest to interpret the user given nodata 
option by writing to metadata (double) (GDAL datatype) . 
No more objections from my side, but I would print a warning if (nodata 
!= (double) (GDAL data type) nodata), telling the user what the nodata 
value will be after GDAL converted it. I would suggest to keep writing 
(double)  with the -f flag, no prior cast. In case of the 
-f flag and a mismatch (nodata != (double) (GDAL data type) nodata), a 
message/warning could be printed with both the nodata value written to 
metadata and the value assigned to NULL cells.


I was assuming that not only CELL, but also FCELL and DCELL maps may be 
exported as GDT_UInt32. Then 2^31 can occur in the raster to be 
exported. Actually, I was assuming that any GRASS raster type may be 
exported as any GDAL data type.



Right. But exporting FCELL or DCELL as GDT_UInt32 is inherently lossy. 
  
Just to make it clear: I don't insist on exporting to GDT_UInt32, but 
this is supported by r.out.gdal for all GRASS raster types, so this 
possibility must be considered IMHO, unless GDT_UInt32 becomes disabled.

For DCELL, every possible GDT_UInt32 value may occur in the source
data, so there isn't any "ideal" default no-data value.
  
That's why I first want to check the range of the raster map(s) to be 
exported.
What if the user asks for a nodata value to be out of range, i.e. really 
wants it to be out of range, leading to a mismatch between the nodata 
value in the metadata and the value assigned to NULL cells in the GDAL 
export? I understood that this was requested to be possible.



Hmm. To allow that, you can't cast to the source or destination type,
  
Exactly. That's why I was thinking about e.g. (double) 2147483648 to be 
written to metadata, and not (double) (GDT_Int32) 2147483648. BTW, I can 
still not see why a user would want that, but it can be allowed.

so the user will have to understand the conversion issues even if they
never use an out-of-range value. Ultimately, its either usability or
flexibility.
  
Without the -f flag I would opt for usability, with the -f flag for 
flexibility.
  
If r.out.gdal should support out of range nodata values, and the nodata 
value is read with atof(), 2^31 should stay 2^31and not be cast to 
-2^31. The GUI description of the nodata option says that it is of type 
float (actually it's a double), i.e. suggesting that any value that is 
representable as floating point is ok.


That's a limitation of the parser. If the input is CELL, then the
nodata value which the input data uses isn't any kind of float. It
might be more correct to have separate int/float nodata options.

OTOH, GDAL's no-data value is always a double, even for integer data.
  
I would rather stick to GDAL's method of handling nodata options and try 
to mimic that in r.out.gdal.



I don't have any preference, although I suspect that users will
regularly get bitten by the conversion issues.
  
As above, by default r.out.gdal could write (double) (GDAL datatype) 
 to metadata, with the -f flag no cast to GDAL datatype?
 
The GDAL API documentation says "To clear the nodata value, just set it 
with an "out of range" value." This is currently possible with the -f 
flag, but r.out.gdal then does not give any information on what value is 
assigned to NULL cells, only what nodata value is written to the metadata.



Right. There's only one nodata= option, and it's performing a dual
function: what to store for null cells and what to set GDAL's no-data
value to. If you want to allow a mismatch, you really need two
options.
  
I thought in this case (allow mismatch) it would be enough to write it 
as double to metadata without prior cast to the selected GDAL datatype.


I don't see the point in treating range limits differently to
precision limits. Either the output type is a superset of the input
type (lossless export) or it isn't (lossy export).
  
Easy to change, with feedback to the user as warning in case of lossy 
export.


When exporting to FP types, the default should probably be NaN (if
supported), regardless of whether the source is integer or FP.

Exporting CELL to GDT_Float32 should print a warning if the range of
the input data exceeds +/- 2^24, as it cannot be stored exactly.
  

OK.
_

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-20 Thread Glynn Clements

Markus Metz wrote:

> >> OK, but I have tested r.out.gdal with type = GDT_Int32 and nodata = 
> >> 0x8000. Same result: NULL cells become -2147483648 but the nodata 
> >> value in the metadata says 2147483648 (gdalinfo on the export output),
> >
> > Yes. I know. Running "r.out.gdal ... nodata=0x8000" is
> > misinterpreting what I was saying, as r.out.gdal parses nodata= as a
> > floating-point value.
> >
> > I'm talking about using (GDT_Int32)0x8000 (i.e. -2^31) 
> 
> I think I slowly begin to understand. You suggest to use 
> (GDT_Int32)0x8000 for both the GDAL metadata info and the GDAL 
> raster data although GDAL metadata nodata is double?

Yes; for the latter, the value would be:

(double) (GDT_Int32) 0x8000

IOW, -2147483648.0.

Note that this isn't the same as atof("0x8000"), which would be
positive.

> When using a default nodata value, what I want to get in the exported 
> map is identical values for the metadata nodata value and the value 
> assigned to NULL cells. GDAL metadata nodata value is always double 
> (GDALSetRasterNoDataValue()), the value assigned to NULL cells is of the 
> selected GDAL datatype. So 0x8000 for GDT_Int32 will be 2^31 in 
> metadata and -2^31 in the data if passed as such to the corresponding 
> GDAL functions, (double ) 0x8000 for metadata info and (GDT_Int32) 
> 0x8000 for raster data.

That depends upon how you interpret "0x8000".

(double) (GDT_Int32) 0x8000

is equal to -2147483648.0, while 

atof("0x8000")

is equal to +2147483648.0.

GDT_Int32 itself cannot represent +2^31.

When writing CELL data as GDT_Int32, the default null value should be
(GDT_Int32) 0x8000, as that's the only value which cannot occur in
the source data. The GDAL no-data value should be that value cast to a
double, i.e.:

(double) (GDT_Int32) 0x8000
or:
atof("-0x8000")
or:
-2147483648.0

or any other permutation which produces the same "double" value.

> > or
> > (GDT_UInt32)0x8000 (i.e. +2^31) as appropriate for the destination
> > type.
> 
> I was assuming that not only CELL, but also FCELL and DCELL maps may be 
> exported as GDT_UInt32. Then 2^31 can occur in the raster to be 
> exported. Actually, I was assuming that any GRASS raster type may be 
> exported as any GDAL data type.

Right. But exporting FCELL or DCELL as GDT_UInt32 is inherently lossy. 
For DCELL, every possible GDT_UInt32 value may occur in the source
data, so there isn't any "ideal" default no-data value.

>  For r.out.gdal, there was discussion about not to override user options 
>  and instead issue an error or a warning (going for error now) that the 
>  nodata value is out of range.
>  
> >>> This isn't about overriding user options, but interpreting them
> >>> correctly.
> >> Close to "the module knows better" which is not desirable according to 
> >> Dylan and Moritz.
> >
> > No, the opposite. Writing code which does what the user asks for
> > rather than doing something else which is easier to code.
> 
> What if the user asks for a nodata value to be out of range, i.e. really 
> wants it to be out of range, leading to a mismatch between the nodata 
> value in the metadata and the value assigned to NULL cells in the GDAL 
> export? I understood that this was requested to be possible.

Hmm. To allow that, you can't cast to the source or destination type,
so the user will have to understand the conversion issues even if they
never use an out-of-range value. Ultimately, its either usability or
flexibility.

> >> If r.out.gdal should support out of range nodata values, and the nodata 
> >> value is read with atof(), 2^31 should stay 2^31and not be cast to 
> >> -2^31. The GUI description of the nodata option says that it is of type 
> >> float (actually it's a double), i.e. suggesting that any value that is 
> >> representable as floating point is ok.
> >
> > That's a limitation of the parser. If the input is CELL, then the
> > nodata value which the input data uses isn't any kind of float. It
> > might be more correct to have separate int/float nodata options.
> >
> > OTOH, GDAL's no-data value is always a double, even for integer data.
> 
> I would rather stick to GDAL's method of handling nodata options and try 
> to mimic that in r.out.gdal.

I don't have any preference, although I suspect that users will
regularly get bitten by the conversion issues.

> >>> OTOH, reading as FP allows you to specify +2^31 as the nodata value
> >>> when exporting CELL as UInt32. Maybe it would be better to read as
> >>> double then cast to the destination type.
> >> Hmm, then it would be no longer possible to have an out-of-range nodata 
> >> value in the metadata. I would support that solution, but it was 
> >> explicitly requested to obey all user options when the -f flag is used, 
> >> also writing the nodata value as it is to the metadata. It is then up to 
> >> the user to figure out what happen

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-20 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:
OK, but I have tested r.out.gdal with type = GDT_Int32 and nodata = 
0x8000. Same result: NULL cells become -2147483648 but the nodata 
value in the metadata says 2147483648 (gdalinfo on the export output),


Yes. I know. Running "r.out.gdal ... nodata=0x8000" is
misinterpreting what I was saying, as r.out.gdal parses nodata= as a
floating-point value.

I'm talking about using (GDT_Int32)0x8000 (i.e. -2^31) 
I think I slowly begin to understand. You suggest to use 
(GDT_Int32)0x8000 for both the GDAL metadata info and the GDAL 
raster data although GDAL metadata nodata is double?
When using a default nodata value, what I want to get in the exported 
map is identical values for the metadata nodata value and the value 
assigned to NULL cells. GDAL metadata nodata value is always double 
(GDALSetRasterNoDataValue()), the value assigned to NULL cells is of the 
selected GDAL datatype. So 0x8000 for GDT_Int32 will be 2^31 in 
metadata and -2^31 in the data if passed as such to the corresponding 
GDAL functions, (double ) 0x8000 for metadata info and (GDT_Int32) 
0x8000 for raster data.

or
(GDT_UInt32)0x8000 (i.e. +2^31) as appropriate for the destination
type.
I was assuming that not only CELL, but also FCELL and DCELL maps may be 
exported as GDT_UInt32. Then 2^31 can occur in the raster to be 
exported. Actually, I was assuming that any GRASS raster type may be 
exported as any GDAL data type.


For r.out.gdal, there was discussion about not to override user options 
and instead issue an error or a warning (going for error now) that the 
nodata value is out of range.


This isn't about overriding user options, but interpreting them
correctly.
Close to "the module knows better" which is not desirable according to 
Dylan and Moritz.


No, the opposite. Writing code which does what the user asks for
rather than doing something else which is easier to code.
What if the user asks for a nodata value to be out of range, i.e. really 
wants it to be out of range, leading to a mismatch between the nodata 
value in the metadata and the value assigned to NULL cells in the GDAL 
export? I understood that this was requested to be possible.
If r.out.gdal should support out of range nodata values, and the nodata 
value is read with atof(), 2^31 should stay 2^31and not be cast to 
-2^31. The GUI description of the nodata option says that it is of type 
float (actually it's a double), i.e. suggesting that any value that is 
representable as floating point is ok.


That's a limitation of the parser. If the input is CELL, then the
nodata value which the input data uses isn't any kind of float. It
might be more correct to have separate int/float nodata options.

OTOH, GDAL's no-data value is always a double, even for integer data.
I would rather stick to GDAL's method of handling nodata options and try 
to mimic that in r.out.gdal.



OTOH, reading as FP allows you to specify +2^31 as the nodata value
when exporting CELL as UInt32. Maybe it would be better to read as
double then cast to the destination type.
Hmm, then it would be no longer possible to have an out-of-range nodata 
value in the metadata. I would support that solution, but it was 
explicitly requested to obey all user options when the -f flag is used, 
also writing the nodata value as it is to the metadata. It is then up to 
the user to figure out what happened to the NULL cells, assuming that 
the user knows what happens when using the -f flag.


I can see some point to that, but may be confusing for the user, who
may assume that they're talking about the data's no-data value when
they're actually specifying GDAL's no-data value.
That's why there are warnings if there will be a mismatch between the 
GDAL metadata nodata value and the GDAL raster nodata value (out of 
range test). And having this mismatch is only possible with the -f flag, 
when forcing r.out.gdal to use exactly the given parameters and options. 
The GDAL API documentation says "To clear the nodata value, just set it 
with an "out of range" value." This is currently possible with the -f 
flag, but r.out.gdal then does not give any information on what value is 
assigned to NULL cells, only what nodata value is written to the metadata.



Even so, 0x8000 is still the ideal value for exporting CELL data
to either UInt32 or Int32. You just have to pick the correct
interpretation of it (unsigned int or signed int respectively).
  
The current as well as previous policy of r.out.gdal is to not to 
interpret the nodata value. It is written as is (currently double) to 
metadata, same like e.g. gdal_translate -a_nodata behaves.


It doesn't have any choice about interpreting it. nodataopt->answer is
a char*, while GDALSetRasterNoDataValue() expects a double. There are
several ways to convert a char* to a double; atof() may be the
simplest to implement, but is it the most correct?
No idea. If you can't think if a more correct

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-19 Thread Glynn Clements

Markus Metz wrote:

> >> That would also mean that r.out.gdal with type = GDT_Int32 and nodata = 
> >> 2147483648, NULL cells become -2147483648 but the nodata value in the 
> >> metadata stays 2147483648 (gdalinfo on the export output), which in turn 
> >> means that other software, also QGIS, does not see any nodata cells in 
> >> the export output ->information loss about NULL cells. That's why I 
> >> wouldn't use 0x8000 as default nodata value for GDT_Int32 in 
> >> r.out.gdal even if r.in.gdal has no problems with it.
> >
> > You're misinterpretating my use of "0x8000" here. I'm talking
> > about the C interpretation, which is the (signed) integer value
> > -2147483648.
> 
> OK, but I have tested r.out.gdal with type = GDT_Int32 and nodata = 
> 0x8000. Same result: NULL cells become -2147483648 but the nodata 
> value in the metadata says 2147483648 (gdalinfo on the export output),

Yes. I know. Running "r.out.gdal ... nodata=0x8000" is
misinterpreting what I was saying, as r.out.gdal parses nodata= as a
floating-point value.

I'm talking about using (GDT_Int32)0x8000 (i.e. -2^31) or
(GDT_UInt32)0x8000 (i.e. +2^31) as appropriate for the destination
type.

> >> For r.out.gdal, there was discussion about not to override user options 
> >> and instead issue an error or a warning (going for error now) that the 
> >> nodata value is out of range.
> >> 
> >
> > This isn't about overriding user options, but interpreting them
> > correctly.
> 
> Close to "the module knows better" which is not desirable according to 
> Dylan and Moritz.

No, the opposite. Writing code which does what the user asks for
rather than doing something else which is easier to code.

> > It's debatable whether just using atof(nodataopt->answer) directly is
> > actually a correct interpretation when the input map is CELL, or
> > whether the code should use e.g. atoi(nodataopt->answer) or
> > (CELL)atof(nodataopt->answer) for CELL inputs. Reading as int (or
> > casting to it) will interpret 0x8000 as -2^31, while reading as FP
> > will interpret it as 2^31.
> 
> If r.out.gdal should support out of range nodata values, and the nodata 
> value is read with atof(), 2^31 should stay 2^31and not be cast to 
> -2^31. The GUI description of the nodata option says that it is of type 
> float (actually it's a double), i.e. suggesting that any value that is 
> representable as floating point is ok.

That's a limitation of the parser. If the input is CELL, then the
nodata value which the input data uses isn't any kind of float. It
might be more correct to have separate int/float nodata options.

OTOH, GDAL's no-data value is always a double, even for integer data.

> > OTOH, reading as FP allows you to specify +2^31 as the nodata value
> > when exporting CELL as UInt32. Maybe it would be better to read as
> > double then cast to the destination type.
> 
> Hmm, then it would be no longer possible to have an out-of-range nodata 
> value in the metadata. I would support that solution, but it was 
> explicitly requested to obey all user options when the -f flag is used, 
> also writing the nodata value as it is to the metadata. It is then up to 
> the user to figure out what happened to the NULL cells, assuming that 
> the user knows what happens when using the -f flag.

I can see some point to that, but may be confusing for the user, who
may assume that they're talking about the data's no-data value when
they're actually specifying GDAL's no-data value.

> > Even so, 0x8000 is still the ideal value for exporting CELL data
> > to either UInt32 or Int32. You just have to pick the correct
> > interpretation of it (unsigned int or signed int respectively).
> >   
> The current as well as previous policy of r.out.gdal is to not to 
> interpret the nodata value. It is written as is (currently double) to 
> metadata, same like e.g. gdal_translate -a_nodata behaves.

It doesn't have any choice about interpreting it. nodataopt->answer is
a char*, while GDALSetRasterNoDataValue() expects a double. There are
several ways to convert a char* to a double; atof() may be the
simplest to implement, but is it the most correct?

> Afraid of rewriting large parts of r.out.gdal: can't we just use 
> -2147483648 as default nodata value for Int32

Yes.

> and 0 as default nodata value for UInt32?

No; 0 is a valid data value. Use +2147483648, as that will never occur
in the input data. Or even any value >= 2147483648.

> Getting tired of this UInt32 example, but again: 
> (unsigned int) 0x8000 is 2^31, yes?

Yes.

> Then this value would be right 
> in the middle of the range of UInt32, and can cause data loss when 
> exporting a FCELL raster as UInt32 .

In that case, use (double) 0xU, which will never exist in CELL
or FCELL data.

Maybe the default should depend upon the input type?

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-19 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:
  
I guess a minimum requirement would be that 
something exported with r.out.gdal and then imported again with 
r.in.gdal should be identical (taking into consideration the region 
settings during export) and not have any data loss, be it due to 
clipping data or due to nodata problems.



That's only possible if the exported data is in a format which can
hold the full range of the data. A UInt32 TIFF can't hold the full
range of CELL data (it can't hold any negative values).
  
A UInt32 can hold the full range of a CELL raster map if there are no 
negative values. What you mean is the potential range, what I mean is 
the data range actually present in the raster. Which still means that 
UInt32 doesn't really make sense as GDAL export type for GRASS rasters.
That would also mean that r.out.gdal with type = GDT_Int32 and nodata = 
2147483648, NULL cells become -2147483648 but the nodata value in the 
metadata stays 2147483648 (gdalinfo on the export output), which in turn 
means that other software, also QGIS, does not see any nodata cells in 
the export output ->information loss about NULL cells. That's why I 
wouldn't use 0x8000 as default nodata value for GDT_Int32 in 
r.out.gdal even if r.in.gdal has no problems with it.



You're misinterpretating my use of "0x8000" here. I'm talking
about the C interpretation, which is the (signed) integer value
-2147483648.
  
OK, but I have tested r.out.gdal with type = GDT_Int32 and nodata = 
0x8000. Same result: NULL cells become -2147483648 but the nodata 
value in the metadata says 2147483648 (gdalinfo on the export output), 
and other software including QGIS doesn't see any nodata cells. I 
haven't changed the part of the code of r.out.gdal that writes the 
nodata value to metadata, also not the code that writes the actual 
raster band.
  
For r.out.gdal, there was discussion about not to override user options 
and instead issue an error or a warning (going for error now) that the 
nodata value is out of range.



This isn't about overriding user options, but interpreting them
correctly.
  
Close to "the module knows better" which is not desirable according to 
Dylan and Moritz.

It's debatable whether just using atof(nodataopt->answer) directly is
actually a correct interpretation when the input map is CELL, or
whether the code should use e.g. atoi(nodataopt->answer) or
(CELL)atof(nodataopt->answer) for CELL inputs. Reading as int (or
casting to it) will interpret 0x8000 as -2^31, while reading as FP
will interpret it as 2^31.
  
If r.out.gdal should support out of range nodata values, and the nodata 
value is read with atof(), 2^31 should stay 2^31and not be cast to 
-2^31. The GUI description of the nodata option says that it is of type 
float (actually it's a double), i.e. suggesting that any value that is 
representable as floating point is ok.

OTOH, reading as FP allows you to specify +2^31 as the nodata value
when exporting CELL as UInt32. Maybe it would be better to read as
double then cast to the destination type.
  
Hmm, then it would be no longer possible to have an out-of-range nodata 
value in the metadata. I would support that solution, but it was 
explicitly requested to obey all user options when the -f flag is used, 
also writing the nodata value as it is to the metadata. It is then up to 
the user to figure out what happened to the NULL cells, assuming that 
the user knows what happens when using the -f flag.
  
Currently, all default nodata values are 
within the range of the selected GDAL data type. User-defined nodata 
values are tested whether they are within the range of the selected data 
type, if not, r.out.gdal in trunk and devbr6 exits with an error 
message. r.out.gdal also exits with an error message if the nodata value 
is present in the raster to be exported in order to avoid data loss.
This is a rather strict and conservative approach because of the 
reported problems with nodata in r.out.gdal exports.



Right.

Even so, 0x8000 is still the ideal value for exporting CELL data
to either UInt32 or Int32. You just have to pick the correct
interpretation of it (unsigned int or signed int respectively).
  
The current as well as previous policy of r.out.gdal is to not to 
interpret the nodata value. It is written as is (currently double) to 
metadata, same like e.g. gdal_translate -a_nodata behaves.


Afraid of rewriting large parts of r.out.gdal: can't we just use 
-2147483648 as default nodata value for Int32 and 0 as default nodata 
value for UInt32? Getting tired of this UInt32 example, but again: 
(unsigned int) 0x8000 is 2^31, yes? Then this value would be right 
in the middle of the range of UInt32, and can cause data loss when 
exporting a FCELL raster as UInt32 . For the sake of the argument, let's 
assume this FCELL raster has only positive values, some of them > 2^31, 
and any errors when casting from FCELL to UInt32 are acknowledged and 

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-18 Thread Glynn Clements

Markus Metz wrote:

> This is now a mix of r.in.gdal and r.out.gdal. The two modules 
> complement each other, and I guess a minimum requirement would be that 
> something exported with r.out.gdal and then imported again with 
> r.in.gdal should be identical (taking into consideration the region 
> settings during export) and not have any data loss, be it due to 
> clipping data or due to nodata problems.

That's only possible if the exported data is in a format which can
hold the full range of the data. A UInt32 TIFF can't hold the full
range of CELL data (it can't hold any negative values).

> >>> The only "fool-proof" nodata value for CELL is 0x8000; all other
> >>> values may be actual data values.
> >>>   
> >> For grass rasters of type CELL, not for gdal export as Int32. gdalinfo 
> >> says NoData Value=2147483648, but cells that should be nodata are 
> >> -2147483648 and are thus not recognized as nodata.
> >> 
> >
> > It worked as of a few days ago, in the version I have checked out:
> >
> > raster/r.in.gdal/main.c:780:
> >
> >if (((GInt32 *) cell)[indx] == (GInt32) dfNoData) {
> >
> > (GInt32)2147483648 and (GInt32)-2147483648 are equal.
>
> I think the problem is that gdal allows the nodata value to be out of 
> range with regard to the raster type,

Yes.

> e.g. in gdal, a nodata value of 
> - with GDT_Byte is legal and can be used to effectively disable 
> nodata information. No idea if this conforms to GeoTIFF specs (probably 
> yes), but I'm pretty sure that this is the current policy of gdal 
> (possible with e.g. gdal_translate).
> That would also mean that r.out.gdal with type = GDT_Int32 and nodata = 
> 2147483648, NULL cells become -2147483648 but the nodata value in the 
> metadata stays 2147483648 (gdalinfo on the export output), which in turn 
> means that other software, also QGIS, does not see any nodata cells in 
> the export output ->information loss about NULL cells. That's why I 
> wouldn't use 0x8000 as default nodata value for GDT_Int32 in 
> r.out.gdal even if r.in.gdal has no problems with it.

You're misinterpretating my use of "0x8000" here. I'm talking
about the C interpretation, which is the (signed) integer value
-2147483648.

> > If it's been changed, use nodata=-0x8000. 
> 
> That's currently done in r.out.gdal in trunk and devbr6.

> > Or cast the nodata value
> > to the source data type then back to double.
> 
> For r.out.gdal, there was discussion about not to override user options 
> and instead issue an error or a warning (going for error now) that the 
> nodata value is out of range.

This isn't about overriding user options, but interpreting them
correctly.

It's debatable whether just using atof(nodataopt->answer) directly is
actually a correct interpretation when the input map is CELL, or
whether the code should use e.g. atoi(nodataopt->answer) or
(CELL)atof(nodataopt->answer) for CELL inputs. Reading as int (or
casting to it) will interpret 0x8000 as -2^31, while reading as FP
will interpret it as 2^31.

OTOH, reading as FP allows you to specify +2^31 as the nodata value
when exporting CELL as UInt32. Maybe it would be better to read as
double then cast to the destination type.

> Currently, all default nodata values are 
> within the range of the selected GDAL data type. User-defined nodata 
> values are tested whether they are within the range of the selected data 
> type, if not, r.out.gdal in trunk and devbr6 exits with an error 
> message. r.out.gdal also exits with an error message if the nodata value 
> is present in the raster to be exported in order to avoid data loss.
> This is a rather strict and conservative approach because of the 
> reported problems with nodata in r.out.gdal exports.

Right.

Even so, 0x8000 is still the ideal value for exporting CELL data
to either UInt32 or Int32. You just have to pick the correct
interpretation of it (unsigned int or signed int respectively).

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-17 Thread Markus Metz


Moritz Lennert wrote:


In the current state is there a possibility of using gdal's acceptance 
of unvalid nodata ? And can we force a nodata value which exists in 
the map ?


I would agree with Dylan that this kind of brute-force method should 
remain possible, be it through the suggested -f flag. I'm always wary 
of programs that assume they know better than you what you want ;-)
Added -f flag to force export in trunk r36769 and devbr6 r36770. With 
the -f flag, r.out.gdal overrides the new safety checks, but still warns 
if thinks there may be problems. Scripts will work as before, but raster 
export will be aborted if any of the safety checks is not passed 
(supposed to be a bugfix, not an enhancement).


I guess this ticket can only be closed after backporting to 6.4.

Markus M
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-17 Thread Moritz Lennert

On 17/04/09 16:53, Markus Metz wrote:
For r.out.gdal, there was discussion about not to override user options 
and instead issue an error or a warning (going for error now) that the 
nodata value is out of range. Currently, all default nodata values are 
within the range of the selected GDAL data type. User-defined nodata 
values are tested whether they are within the range of the selected data 
type, if not, r.out.gdal in trunk and devbr6 exits with an error 
message. r.out.gdal also exits with an error message if the nodata value 
is present in the raster to be exported in order to avoid data loss.
This is a rather strict and conservative approach because of the 
reported problems with nodata in r.out.gdal exports.


In the current state is there a possibility of using gdal's acceptance 
of unvalid nodata ? And can we force a nodata value which exists in the 
map ?


I would agree with Dylan that this kind of brute-force method should 
remain possible, be it through the suggested -f flag. I'm always wary of 
programs that assume they know better than you what you want ;-)


Moritz
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-17 Thread Markus Metz
This is now a mix of r.in.gdal and r.out.gdal. The two modules 
complement each other, and I guess a minimum requirement would be that 
something exported with r.out.gdal and then imported again with 
r.in.gdal should be identical (taking into consideration the region 
settings during export) and not have any data loss, be it due to 
clipping data or due to nodata problems.


Glynn Clements wrote:
  

The only "fool-proof" nodata value for CELL is 0x8000; all other
values may be actual data values.
  
For grass rasters of type CELL, not for gdal export as Int32. gdalinfo 
says NoData Value=2147483648, but cells that should be nodata are 
-2147483648 and are thus not recognized as nodata.



It worked as of a few days ago, in the version I have checked out:

raster/r.in.gdal/main.c:780:

   if (((GInt32 *) cell)[indx] == (GInt32) dfNoData) {

(GInt32)2147483648 and (GInt32)-2147483648 are equal.
  
I think the problem is that gdal allows the nodata value to be out of 
range with regard to the raster type, e.g. in gdal, a nodata value of 
- with GDT_Byte is legal and can be used to effectively disable 
nodata information. No idea if this conforms to GeoTIFF specs (probably 
yes), but I'm pretty sure that this is the current policy of gdal 
(possible with e.g. gdal_translate).
That would also mean that r.out.gdal with type = GDT_Int32 and nodata = 
2147483648, NULL cells become -2147483648 but the nodata value in the 
metadata stays 2147483648 (gdalinfo on the export output), which in turn 
means that other software, also QGIS, does not see any nodata cells in 
the export output ->information loss about NULL cells. That's why I 
wouldn't use 0x8000 as default nodata value for GDT_Int32 in 
r.out.gdal even if r.in.gdal has no problems with it.
If it's been changed, use nodata=-0x8000. 

That's currently done in r.out.gdal in trunk and devbr6.

Or cast the nodata value
to the source data type then back to double.
  
For r.out.gdal, there was discussion about not to override user options 
and instead issue an error or a warning (going for error now) that the 
nodata value is out of range. Currently, all default nodata values are 
within the range of the selected GDAL data type. User-defined nodata 
values are tested whether they are within the range of the selected data 
type, if not, r.out.gdal in trunk and devbr6 exits with an error 
message. r.out.gdal also exits with an error message if the nodata value 
is present in the raster to be exported in order to avoid data loss.
This is a rather strict and conservative approach because of the 
reported problems with nodata in r.out.gdal exports.


Markus M

___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-16 Thread Glynn Clements

Markus Metz wrote:

> >>> If you're exporting to a 32-bit (signed or unsigned) integer format,
> >>> the obvious choice for null is 0x8000. That's what GRASS uses
> >>> internally, so it can never occur as a legitimate value (if a
> >>> computation ends up producing that value, it will get treated as a
> >>> null in all regards).
> >>>   
> >>>   
> >> 0x8000 as nodata value seems to work with GeoTIFF for Int32 but not 
> >> for UInt32, here gdalinfo tells me NoData Value=2147483648 which is well 
> >> within the range of UInt32. It may be a problem that nodataval in 
> >> r.out.gdal is of type double, so 0x8000 is cast to double and then 
> >> either back to int or left as double. In the case of UInt32, the raster 
> >> to be exported is currently read as DCELL, written as GDT_Float64 and 
> >> then converted to GDT_UInt32. There is a lot of type casting going on in 
> >> r.out.gdal, making tests a bit complicated.
> >> 
> >
> > Nope.
>
> Not sure if your Nope refers to all of the above or only some of the 
> above ;-)

To the guesses about int/float casts being involved, or r.out.gdal
being involved. Export works fine either way; it's the import that's
problematic.

> Int32 is preferable over UInt32, ok, but it is possible to export a CELL 
> (or any other) raster as UInt32.

It's possible, but it's lossy, the same as exporting CELL to an 8-bit
or 16-bit format.

> > If the input is GDT_Int32, GDAL interprets 0x8000 as -2147483648,
> > which is within the range of an int. But if the input is GDT_UInt32,
> > 0x8000 is interpreted as +2147483648, which is out of range.
> >   
> out of range of CELL, not GDT_UInt32.

Yes.

> Input can not be GDT_UInt32, 
> unless you are referring to r.in.gdal instead of r.out.gdal (leaving out 
> r.external links).

I'm referring to r.in.gdal.

> > r.in.gdal can't just read the data as GDT_UInt32, as that would result
> > in negative values being converted to zero.
> >   
> ??? should r.in.gdal not import data as they are, e.g. UInt32, and then 
> select a grass raster type that can hold the data range, FCELL or DCELL 
> for UInt32, but not CELL?

Arguably it should, but it doesn't; both Int32 and UInt32 are imported
as Int32/CELL.

It should probably check the range of the data to select an
appropriate type. E.g. if it's UInt32 but the range will fit into
CELL, then import as UInt32 into a CELL array.

> >  As CELL is signed, exporting as
> > GDT_UInt32 is lossy; half the range of the source type is
> > unrepresentable, while half the range of the destination type is
> > unused.
> 
> Still, it is possible. Your comment could go into the manual. All the 
> complex GDAL data types are also not represented by GRASS raster types.

Exporting to complex is safe, but somewhat pointless (unless you add
an option to export separate real and imaginary maps; this might be
useful for i.fft output).

> > The only "fool-proof" nodata value for CELL is 0x8000; all other
> > values may be actual data values.
> 
> For grass rasters of type CELL, not for gdal export as Int32. gdalinfo 
> says NoData Value=2147483648, but cells that should be nodata are 
> -2147483648 and are thus not recognized as nodata.

It worked as of a few days ago, in the version I have checked out:

raster/r.in.gdal/main.c:780:

   if (((GInt32 *) cell)[indx] == (GInt32) dfNoData) {

(GInt32)2147483648 and (GInt32)-2147483648 are equal.

If it's been changed, use nodata=-0x8000. Or cast the nodata value
to the source data type then back to double.

> I could not find a fool-proof nodata value for Int32 exports in my tests 
> (independent of the grass raster map type), so I would leave it to the 
> user and within r.out.gdal make sure the selected nodata value is not 
> present in the grass raster.
> 
> Markus M
> 
> PS: I don't insist on exporting as UInt32, that was just an example. A 
> better example would have been the troublemaker UInt16.

For an 8-bit or 16-bit type, overflow can't always be avoided. But it
should always be possible to losslessly export CELL to a 32-bit type.

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-16 Thread Markus Metz


Glynn Clements wrote:

Markus Metz wrote:

  
3. Allow the user to specify what they would like NULL cells encoded as. 
Unless I am overlooking something, it would seem reasonable to export a CELL 
map to a signed integer format, and use some obvious negative value for NULL:



If you're exporting to a 32-bit (signed or unsigned) integer format,
the obvious choice for null is 0x8000. That's what GRASS uses
internally, so it can never occur as a legitimate value (if a
computation ends up producing that value, it will get treated as a
null in all regards).
  
  
0x8000 as nodata value seems to work with GeoTIFF for Int32 but not 
for UInt32, here gdalinfo tells me NoData Value=2147483648 which is well 
within the range of UInt32. It may be a problem that nodataval in 
r.out.gdal is of type double, so 0x8000 is cast to double and then 
either back to int or left as double. In the case of UInt32, the raster 
to be exported is currently read as DCELL, written as GDT_Float64 and 
then converted to GDT_UInt32. There is a lot of type casting going on in 
r.out.gdal, making tests a bit complicated.



Nope.
  
Not sure if your Nope refers to all of the above or only some of the 
above ;-)
Int32 is preferable over UInt32, ok, but it is possible to export a CELL 
(or any other) raster as UInt32. With regard to the type casting, in 
trunk, 0x8000 would be cast to double:

http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.out.gdal/main.c#L306
and then to UInt32, only replaced NULL cells, not the metadata nodata value:
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.out.gdal/main.c#L440



If the input is GDT_Int32, GDAL interprets 0x8000 as -2147483648,
which is within the range of an int. But if the input is GDT_UInt32,
0x8000 is interpreted as +2147483648, which is out of range.
  
out of range of CELL, not GDT_UInt32. Input can not be GDT_UInt32, 
unless you are referring to r.in.gdal instead of r.out.gdal (leaving out 
r.external links). If your Nope refers to 0x8000 as working nodata 
value for Int32, you're right, I was too fast. gdal (in r.out.gdal with 
export type Int32) does conversions of 0x8000 replacing NULL cells 
but not of 0x8000 as the nodata value in the metadata -> information 
loss about what cells are nodata.

r.in.gdal can't just read the data as GDT_UInt32, as that would result
in negative values being converted to zero.
  
??? should r.in.gdal not import data as they are, e.g. UInt32, and then 
select a grass raster type that can hold the data range, FCELL or DCELL 
for UInt32, but not CELL?

Ultimately, exporting CELL data as GDT_UInt32 is a bad idea, as it
cannot represent negative values.
The new version of r.out.gdal checks if the grass raster data fit into 
the selected GDAL data type, if not, aborts with a message telling you 
the range of the raster to be exported and the range of the selected 
GDAL data type. It is then easy to figure out an appropriate GDAL data 
type with the help of the manual.

 As CELL is signed, exporting as
GDT_UInt32 is lossy; half the range of the source type is
unrepresentable, while half the range of the destination type is
unused.
  
Still, it is possible. Your comment could go into the manual. All the 
complex GDAL data types are also not represented by GRASS raster types.


The only "fool-proof" nodata value for CELL is 0x8000; all other
values may be actual data values.
  
For grass rasters of type CELL, not for gdal export as Int32. gdalinfo 
says NoData Value=2147483648, but cells that should be nodata are 
-2147483648 and are thus not recognized as nodata.

More generally, you can't losslessly export N+1 data values using a
format which only supports N values (e.g. exporting 0-255 plus null
using only one byte per cell).
  
As above, this is checked against in trunk and devbr6 and no longer 
possible.


I could not find a fool-proof nodata value for Int32 exports in my tests 
(independent of the grass raster map type), so I would leave it to the 
user and within r.out.gdal make sure the selected nodata value is not 
present in the grass raster.


Markus M

PS: I don't insist on exporting as UInt32, that was just an example. A 
better example would have been the troublemaker UInt16.

___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-16 Thread Glynn Clements

Markus Metz wrote:

> >> 3. Allow the user to specify what they would like NULL cells encoded as. 
> >> Unless I am overlooking something, it would seem reasonable to export a 
> >> CELL 
> >> map to a signed integer format, and use some obvious negative value for 
> >> NULL:
> >> 
> >
> > If you're exporting to a 32-bit (signed or unsigned) integer format,
> > the obvious choice for null is 0x8000. That's what GRASS uses
> > internally, so it can never occur as a legitimate value (if a
> > computation ends up producing that value, it will get treated as a
> > null in all regards).
> >   
> 0x8000 as nodata value seems to work with GeoTIFF for Int32 but not 
> for UInt32, here gdalinfo tells me NoData Value=2147483648 which is well 
> within the range of UInt32. It may be a problem that nodataval in 
> r.out.gdal is of type double, so 0x8000 is cast to double and then 
> either back to int or left as double. In the case of UInt32, the raster 
> to be exported is currently read as DCELL, written as GDT_Float64 and 
> then converted to GDT_UInt32. There is a lot of type casting going on in 
> r.out.gdal, making tests a bit complicated.

Nope.

The problem is that r.in.gdal reads the data as GDT_Int32, which
causes GDAL to clamp 0x8000 to 0x7fff.

If the input is GDT_Int32, GDAL interprets 0x8000 as -2147483648,
which is within the range of an int. But if the input is GDT_UInt32,
0x8000 is interpreted as +2147483648, which is out of range.

r.in.gdal can't just read the data as GDT_UInt32, as that would result
in negative values being converted to zero.

Ultimately, exporting CELL data as GDT_UInt32 is a bad idea, as it
cannot represent negative values. As CELL is signed, exporting as
GDT_UInt32 is lossy; half the range of the source type is
unrepresentable, while half the range of the destination type is
unused.

> In the end, the value 
> assigned to NULL cells must match the nodata value in the metadata, and 
> a safe default choice seems to be type min or type max.
> 
> NaN seems to work for Float32 and Float64, at least gdal reports NoData 
> Value=nan and QGIS display is ok for GTiff, HFA, PAux, EHdr. NaN is 
> constructed with 0.0/0.0, idea from r.univar.
> 
> IMHO, using 0x8000 or similar can be dangerous (not sure about NaN) 
> because I don't know how these values are supported by all the different 
> file formats and all the different applications. Testing welcome!
> 
> I would suggest to use some fool-proof default nodata value for GDAL 
> integer types, i.e. type min or type max, check if it works, otherwise 
> abort and ask the user to specify a nodata value. For GDAL floating 
> point types, it could be recommended to use the default nodata value 
> (NaN) instead of specifying a nodata value.

The only "fool-proof" nodata value for CELL is 0x8000; all other
values may be actual data values.

More generally, you can't losslessly export N+1 data values using a
format which only supports N values (e.g. exporting 0-255 plus null
using only one byte per cell).

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-16 Thread Markus Metz


Glynn Clements wrote:

Dylan Beaudette wrote:

  
3. Allow the user to specify what they would like NULL cells encoded as. 
Unless I am overlooking something, it would seem reasonable to export a CELL 
map to a signed integer format, and use some obvious negative value for NULL:



If you're exporting to a 32-bit (signed or unsigned) integer format,
the obvious choice for null is 0x8000. That's what GRASS uses
internally, so it can never occur as a legitimate value (if a
computation ends up producing that value, it will get treated as a
null in all regards).
  
0x8000 as nodata value seems to work with GeoTIFF for Int32 but not 
for UInt32, here gdalinfo tells me NoData Value=2147483648 which is well 
within the range of UInt32. It may be a problem that nodataval in 
r.out.gdal is of type double, so 0x8000 is cast to double and then 
either back to int or left as double. In the case of UInt32, the raster 
to be exported is currently read as DCELL, written as GDT_Float64 and 
then converted to GDT_UInt32. There is a lot of type casting going on in 
r.out.gdal, making tests a bit complicated. In the end, the value 
assigned to NULL cells must match the nodata value in the metadata, and 
a safe default choice seems to be type min or type max.


NaN seems to work for Float32 and Float64, at least gdal reports NoData 
Value=nan and QGIS display is ok for GTiff, HFA, PAux, EHdr. NaN is 
constructed with 0.0/0.0, idea from r.univar.


IMHO, using 0x8000 or similar can be dangerous (not sure about NaN) 
because I don't know how these values are supported by all the different 
file formats and all the different applications. Testing welcome!


I would suggest to use some fool-proof default nodata value for GDAL 
integer types, i.e. type min or type max, check if it works, otherwise 
abort and ask the user to specify a nodata value. For GDAL floating 
point types, it could be recommended to use the default nodata value 
(NaN) instead of specifying a nodata value.


Markus M
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-15 Thread Dylan Beaudette
On Wednesday 15 April 2009, Glynn Clements wrote:
> Dylan Beaudette wrote:
> > 3. Allow the user to specify what they would like NULL cells encoded as.
> > Unless I am overlooking something, it would seem reasonable to export a
> > CELL map to a signed integer format, and use some obvious negative value
> > for NULL:
>
> If you're exporting to a 32-bit (signed or unsigned) integer format,
> the obvious choice for null is 0x8000. That's what GRASS uses
> internally, so it can never occur as a legitimate value (if a
> computation ends up producing that value, it will get treated as a
> null in all regards).

Thanks Glynn. It would be good to know about 'obvious' values for any given 
datatype.

Cheers,
Dylan


-- 
Dylan Beaudette
Soil Resource Laboratory
http://casoilresource.lawr.ucdavis.edu/
University of California at Davis
530.754.7341
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-15 Thread Glynn Clements

Dylan Beaudette wrote:

> 3. Allow the user to specify what they would like NULL cells encoded as. 
> Unless I am overlooking something, it would seem reasonable to export a CELL 
> map to a signed integer format, and use some obvious negative value for NULL:

If you're exporting to a 32-bit (signed or unsigned) integer format,
the obvious choice for null is 0x8000. That's what GRASS uses
internally, so it can never occur as a legitimate value (if a
computation ends up producing that value, it will get treated as a
null in all regards).

-- 
Glynn Clements 
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-15 Thread Markus Metz


Moritz Lennert wrote:

On 15/04/09 19:22, Dylan Beaudette wrote:

On Wednesday 15 April 2009, GRASS GIS wrote:

#73: r.out.gdal tiff output does not work
--+ 

- Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org 
Type: defect   |  Status:  new

  Priority:  critical |   Milestone:  6.4.0
 Component:  Raster   | Version:  svn-trunk
Resolution:   |Keywords:  r.out.gdal, tiff
  Platform:  Unspecified  | Cpu:  Unspecified
--+ 


- Comment (by hamish):

 Replying to [comment:32 mmetz]:
 > My patch only adds safety checks to make sure that the selected
 > GDAL data type covers the range of the raster band to be exported
 > and that the given null value is within the range of the selected
 > GDAL data type. That would be an enhancement, not a bugfix.

 Pretend you are r.out.gdal iterating through the raster cells to 
output.
 You come across a NULL cell. You have to write down some value, and 
are
 completely constrained to options available in the output type. 
What do
 you write out? Overflow/wrap the type? type min? type max? well (as 
things

 were) whatever you do is going to be arbirtary, and thus could lead to
 data loss, which is grounds for a most serious blocker bug.


I haven't thought a great deal about this, but from what I have been 
reading and recalling past messages on the GDAL mailing list-- there 
just aren't any general approaches to preserving NULL values in a 
geotiff when you need the entire range of a given datatype to 
represent your information.
I think that several things can be done to address this bug, some of 
which have been mentioned:


1. if there are any NULL cells, warn the user- possibly refuse to 
export the image in the current format (BYTE, etc.)


1a. if the selected output format cannot contain the entire range, or 
precision, of input map data (input is FCELL, output is BYTE) give a 
warning or fail with error message.


2. include a 'force' option to make r.out.gdal do exactly what your 
command line instructions suggest-- possibly leading to corruption of 
NULL data


3. Allow the user to specify what they would like NULL cells encoded 
as. Unless I am overlooking something, it would seem reasonable to 
export a CELL map to a signed integer format, and use some obvious 
negative value for NULL:


# assuming that all REAL data is within 0-255
r.out.gdal in=CELLMAP out=map.tif format=SIGNEDINT null=-1



 > also, because the manual has been updated.

 that is easily modified, so shouldn't stop us.

 > In any case, it would be great if this ticket could be solved
 > before the final release of 6.4.

 yup



I would like to see the above changes, however I cannot actually 
implement them. Thanks to everyone who has been working on this 
important module.


AFAIK, except for number 1, all are implemented already. Currently 
there is an error if the null value does not fit into the data range 
of the selected output file type, but not if the selected value for 
null data is present in the data as a valid value. But as Markus 
explained, checking for this would be quite intense, so IMHO a warning 
that there are null values and that there is the possibility of 
loosing information if the user does not watch out should be enough.
1. warn the user only if no nodata value is given, otherwise (nodata 
value was given) assume that the user knows what she/he is doing?

1a. only in the patch which is not submitted, and no precision test.
2. suggested but nowhere implemented. why should we have a force option 
to override all the requested but not yet present safety checks?

3. was always possible

I THINK calculating a reasonable nodata value is both time and memory 
expensive, maybe there is some ingenious way to avoid memory costs, but 
I'm pretty sure that two passes for each raster band are necessary, i.e. 
time intensive.


r.out.gdal selects a file format, a suitable data type and a default 
nodata value if none of them were given, but that's more of a guess, and 
the user should know best what file format, data type and nodata value 
is appropriate. It gets all a bit more complicated because r.out.gdal 
can export several raster maps into one single file if supported by the 
selected file format. Therefore I would prefer some simple tests and 
abort with a helpful message if something goes wrong instead of doing 
fancy automated guessing that would make the code much more complex and 
thus more difficult to maintain.


r.out.gdal supports all raster formats supported by gdal, and that's a 
lot, becoming more with every new version of gdal. IMHO, r.out.gdal 
should check if input options would cause corrupted output and otherwise 
refer to the gdal documentation.


I'm busy working on an update for r.out.gdal and should be able to 
submit to trunk tomorrow. Changes are range-

Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-15 Thread Dylan Beaudette
On Wednesday 15 April 2009, Moritz Lennert wrote:
> On 15/04/09 19:22, Dylan Beaudette wrote:
> > On Wednesday 15 April 2009, GRASS GIS wrote:
> >> #73: r.out.gdal tiff output does not work
> >> --+-
> >>--- - Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org
> >> Type: defect   |  Status:  new
> >>   Priority:  critical |   Milestone:  6.4.0
> >>  Component:  Raster   | Version:  svn-trunk
> >> Resolution:   |Keywords:  r.out.gdal, tiff
> >>   Platform:  Unspecified  | Cpu:  Unspecified
> >> --+-
> >>--- - Comment (by hamish):
> >>
> >>  Replying to [comment:32 mmetz]:
> >>  > My patch only adds safety checks to make sure that the selected
> >>  > GDAL data type covers the range of the raster band to be exported
> >>  > and that the given null value is within the range of the selected
> >>  > GDAL data type. That would be an enhancement, not a bugfix.
> >>
> >>  Pretend you are r.out.gdal iterating through the raster cells to
> >> output. You come across a NULL cell. You have to write down some value,
> >> and are completely constrained to options available in the output type.
> >> What do you write out? Overflow/wrap the type? type min? type max? well
> >> (as things were) whatever you do is going to be arbirtary, and thus
> >> could lead to data loss, which is grounds for a most serious blocker
> >> bug.
> >
> > I haven't thought a great deal about this, but from what I have been
> > reading and recalling past messages on the GDAL mailing list-- there just
> > aren't any general approaches to preserving NULL values in a geotiff when
> > you need the entire range of a given datatype to represent your
> > information.
> >
> > I think that several things can be done to address this bug, some of
> > which have been mentioned:
> >
> > 1. if there are any NULL cells, warn the user- possibly refuse to export
> > the image in the current format (BYTE, etc.)
> >
> > 1a. if the selected output format cannot contain the entire range, or
> > precision, of input map data (input is FCELL, output is BYTE) give a
> > warning or fail with error message.
> >
> > 2. include a 'force' option to make r.out.gdal do exactly what your
> > command line instructions suggest-- possibly leading to corruption of
> > NULL data
> >
> > 3. Allow the user to specify what they would like NULL cells encoded as.
> > Unless I am overlooking something, it would seem reasonable to export a
> > CELL map to a signed integer format, and use some obvious negative value
> > for NULL:
> >
> > # assuming that all REAL data is within 0-255
> > r.out.gdal in=CELLMAP out=map.tif format=SIGNEDINT null=-1
> >
> >>  > also, because the manual has been updated.
> >>
> >>  that is easily modified, so shouldn't stop us.
> >>
> >>  > In any case, it would be great if this ticket could be solved
> >>  > before the final release of 6.4.
> >>
> >>  yup
> >
> > I would like to see the above changes, however I cannot actually
> > implement them. Thanks to everyone who has been working on this important
> > module.
>
> AFAIK, except for number 1, all are implemented already. Currently there
> is an error if the null value does not fit into the data range of the
> selected output file type, but not if the selected value for null data
> is present in the data as a valid value. But as Markus explained,
> checking for this would be quite intense, so IMHO a warning that there
> are null values and that there is the possibility of loosing information
> if the user does not watch out should be enough.
>
> Moritz

Thanks for the notice on 2-3. Your approach sounds good to me. 

Cheers,
Dylan

-- 
Dylan Beaudette
Soil Resource Laboratory
http://casoilresource.lawr.ucdavis.edu/
University of California at Davis
530.754.7341
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2009-04-15 Thread Moritz Lennert

On 15/04/09 19:22, Dylan Beaudette wrote:

On Wednesday 15 April 2009, GRASS GIS wrote:

#73: r.out.gdal tiff output does not work
--+
- Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org Type: 
defect   |  Status:  new

  Priority:  critical |   Milestone:  6.4.0
 Component:  Raster   | Version:  svn-trunk
Resolution:   |Keywords:  r.out.gdal, tiff
  Platform:  Unspecified  | Cpu:  Unspecified
--+
- Comment (by hamish):

 Replying to [comment:32 mmetz]:
 > My patch only adds safety checks to make sure that the selected
 > GDAL data type covers the range of the raster band to be exported
 > and that the given null value is within the range of the selected
 > GDAL data type. That would be an enhancement, not a bugfix.

 Pretend you are r.out.gdal iterating through the raster cells to output.
 You come across a NULL cell. You have to write down some value, and are
 completely constrained to options available in the output type. What do
 you write out? Overflow/wrap the type? type min? type max? well (as things
 were) whatever you do is going to be arbirtary, and thus could lead to
 data loss, which is grounds for a most serious blocker bug.


I haven't thought a great deal about this, but from what I have been reading 
and recalling past messages on the GDAL mailing list-- there just aren't any 
general approaches to preserving NULL values in a geotiff when you need the 
entire range of a given datatype to represent your information. 

I think that several things can be done to address this bug, some of which 
have been mentioned:


1. if there are any NULL cells, warn the user- possibly refuse to export the 
image in the current format (BYTE, etc.)


1a. if the selected output format cannot contain the entire range, or 
precision, of input map data (input is FCELL, output is BYTE) give a warning 
or fail with error message.


2. include a 'force' option to make r.out.gdal do exactly what your command 
line instructions suggest-- possibly leading to corruption of NULL data


3. Allow the user to specify what they would like NULL cells encoded as. 
Unless I am overlooking something, it would seem reasonable to export a CELL 
map to a signed integer format, and use some obvious negative value for NULL:


# assuming that all REAL data is within 0-255
r.out.gdal in=CELLMAP out=map.tif format=SIGNEDINT null=-1



 > also, because the manual has been updated.

 that is easily modified, so shouldn't stop us.

 > In any case, it would be great if this ticket could be solved
 > before the final release of 6.4.

 yup



I would like to see the above changes, however I cannot actually implement 
them. Thanks to everyone who has been working on this important module.


AFAIK, except for number 1, all are implemented already. Currently there 
is an error if the null value does not fit into the data range of the 
selected output file type, but not if the selected value for null data 
is present in the data as a valid value. But as Markus explained, 
checking for this would be quite intense, so IMHO a warning that there 
are null values and that there is the possibility of loosing information 
if the user does not watch out should be enough.


Moritz
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2008-10-24 Thread Dylan Beaudette
On Friday 24 October 2008, Helena Mitasova wrote:
> On Oct 24, 2008, at 9:32 AM, Moritz Lennert wrote:
> > On 24/10/08 14:10, Markus Metz wrote:
> >> GRASS GIS wrote:
> >>> #73: r.out.gdal tiff output does not work
> >>> --
> >>> +-
> >>>   Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org
> >>>   Type:  defect   |  Status:
> >>> newPriority:  critical |
> >>> Milestone:  6.4.0 Component:  Raster
> >>>
> >>> | Version:  svn-trunk
> >>>
> >>> Resolution:   |Keywords:  r.out.gdal,
> >>> tiff   Platform:  Unspecified  | Cpu:
> >>> Unspecified  --
> >>> +-
> >>> Comment (by neteler):
> >>>
> >>>  Markus (Metz),
> >>>
> >>>  what about integrating your fixes from
> >>>   http://markus.metz.giswork.googlepages.com/
> >>> r.out.gdal.conservative.tar.gz
> >>>  ?
> >>>
> >>>  Markus
> >>
> >> Sure, no objections from my side, I'm using this version only. But
> >> r.out.gdal is a very important module of GRASS and maybe some more
> >> testing is required. Also, the new features may not find approval by
> >> all, e.g. I've put in rather restrictive NULL cell handling: the user
> >> has to specify a nodata value that falls within the range of the
> >> selected datatype (Byte, UInt16, Int16, ...), a nodata value is no
> >> longer chosen automatically as in the original version. If a nodata
> >> value is not given, but NULL cells are present, r.out.gdal aborts
> >> with
> >> an error message. My version also no longer uses the current region
> >> resolution, instead the current region extends are aligned to the
> >> resolution of the raster to be exported to avoid any implicit
> >> resampling. And the colortable is only exported on request, and then
> >> with a warning message.
> >> If these changes are ok with you then integrate the changes,
> >> otherwise
> >> maybe only some, but not all changes could be integrated. More
> >> discussion needed on how to change/improve r.out.gdal?
> >
> > +1 for all of these changes. Let's try with those and then discuss.
> >
> > Moritz
>
> I think those are useful changes and agree with Moritz that we should
> try them out.
> maybe add it to svn as r.out.gdal2 so that we can test and compare
> with the old one
> and then replace if there are no problems? It would be a huge help to
> get this resolved,
>
> Helena

+1 from me. I once it is in the 64 branch I can do some testing.

Dylan


> > ___
> > grass-dev mailing list
> > grass-dev@lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/grass-dev
>
> ___
> grass-dev mailing list
> grass-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/grass-dev



-- 
Dylan Beaudette
Soil Resource Laboratory
http://casoilresource.lawr.ucdavis.edu/
University of California at Davis
530.754.7341
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2008-10-24 Thread Helena Mitasova


On Oct 24, 2008, at 9:32 AM, Moritz Lennert wrote:


On 24/10/08 14:10, Markus Metz wrote:

GRASS GIS wrote:

#73: r.out.gdal tiff output does not work
-- 
+-

  Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org
  Type:  defect   |  Status:   
newPriority:  critical |
Milestone:  6.4.0 Component:  Raster
| Version:  svn-trunk 
Resolution:   |Keywords:  r.out.gdal,  
tiff   Platform:  Unspecified  | Cpu:   
Unspecified  -- 
+-

Comment (by neteler):

 Markus (Metz),

 what about integrating your fixes from
  http://markus.metz.giswork.googlepages.com/ 
r.out.gdal.conservative.tar.gz

 ?

 Markus



Sure, no objections from my side, I'm using this version only. But
r.out.gdal is a very important module of GRASS and maybe some more
testing is required. Also, the new features may not find approval by
all, e.g. I've put in rather restrictive NULL cell handling: the user
has to specify a nodata value that falls within the range of the
selected datatype (Byte, UInt16, Int16, ...), a nodata value is no
longer chosen automatically as in the original version. If a nodata
value is not given, but NULL cells are present, r.out.gdal aborts  
with

an error message. My version also no longer uses the current region
resolution, instead the current region extends are aligned to the
resolution of the raster to be exported to avoid any implicit
resampling. And the colortable is only exported on request, and then
with a warning message.
If these changes are ok with you then integrate the changes,  
otherwise

maybe only some, but not all changes could be integrated. More
discussion needed on how to change/improve r.out.gdal?


+1 for all of these changes. Let's try with those and then discuss.

Moritz


I think those are useful changes and agree with Moritz that we should  
try them out.
maybe add it to svn as r.out.gdal2 so that we can test and compare  
with the old one
and then replace if there are no problems? It would be a huge help to  
get this resolved,


Helena

___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2008-10-24 Thread Moritz Lennert

On 24/10/08 14:10, Markus Metz wrote:


GRASS GIS wrote:

#73: r.out.gdal tiff output does not work
--+-
  Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org
  Type:  defect   |  Status:  new  
  Priority:  critical |   Milestone:  6.4.0
 Component:  Raster   | Version:  svn-trunk
Resolution:   |Keywords:  r.out.gdal, tiff 
  Platform:  Unspecified  | Cpu:  Unspecified  
--+-

Comment (by neteler):

 Markus (Metz),

 what about integrating your fixes from
  http://markus.metz.giswork.googlepages.com/r.out.gdal.conservative.tar.gz
 ?

 Markus

  

Sure, no objections from my side, I'm using this version only. But
r.out.gdal is a very important module of GRASS and maybe some more
testing is required. Also, the new features may not find approval by
all, e.g. I've put in rather restrictive NULL cell handling: the user
has to specify a nodata value that falls within the range of the
selected datatype (Byte, UInt16, Int16, ...), a nodata value is no
longer chosen automatically as in the original version. If a nodata
value is not given, but NULL cells are present, r.out.gdal aborts with
an error message. My version also no longer uses the current region
resolution, instead the current region extends are aligned to the
resolution of the raster to be exported to avoid any implicit
resampling. And the colortable is only exported on request, and then
with a warning message.
If these changes are ok with you then integrate the changes, otherwise
maybe only some, but not all changes could be integrated. More
discussion needed on how to change/improve r.out.gdal?


+1 for all of these changes. Let's try with those and then discuss.

Moritz
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2008-05-08 Thread Glynn Clements

GRASS GIS wrote:

>  Final test which seems to show that there is something wrong: r.in.gdal
>  in=test.tif out=test: import takes a very long time and creates a huge
>  colr table file:
> 
>  % 0 65535
>  0:0
>  1:0
>  2:0
>  3:0
>  4:0
>  5:0
>  6:0
>  7:0
>  8:0
>  9:0
>  10:0
>  11:0
>  [...]
>  65535:0 65535:0
> 
>  Whereas the original colr file only contains this:
>  % 334 460
>  334:0:191:191 359.2:0:255:0
>  359.2:0:255:0 384.4:255:255:0
>  384.4:255:255:0 409.6:255:127:0
>  409.6:255:127:0 434.8:191:127:63
>  434.8:191:127:63 460:20
> 
>  The display of test also takes a long time, I suppose because of this long
>  colr table.

r.out.gdal stores the colour table in a sequence of metadata items
named COLOR_TABLE_RULES_COUNT and COLOR_TABLE_RULE_RGB_.

However, r.in.gdal ignores these settings. It just creates a colour
table which matches the palette of the imported file.

Although that's arguably the right thing to do]1], a 64K-entry table
is less than ideal. For a start, creating a colour table is O(n^2), as
each new entry is checked for overlap against all existing entries.

[1] If the data has been modified, the metadata items may have been
passed through untouched, so will no longer be correct.

One option is to provide an option to create the colour table from the
metadata items rather than from the palette. Another is to
"reverse-engineer" the colour table from the palette, replacing
sequences of palette entries with a single rule.

The latter is preferable, but it's non-trivial, particularly if you
consider that the original rules may have used non-integer endpoints.

-- 
Glynn Clements <[EMAIL PROTECTED]>
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev


Re: [GRASS-dev] Re: [GRASS GIS] #73: r.out.gdal tiff output does not work

2008-03-11 Thread Glynn Clements

GRASS GIS wrote:

> #73: r.out.gdal tiff output does not work
> --+-
>   Reporter:  helena   |   Owner:  grass-dev@lists.osgeo.org
>   Type:  defect   |  Status:  new  
>   Priority:  major|   Milestone:  6.4.0
>  Component:  default  | Version:  svn-trunk
> Resolution:   |Keywords:  r.out.gdal, tiff 
> --+-
> Comment (by hamish):
> 
>  (I see the same with 6.2.1's old script version of r.out.gdal)
> 
>  it works if the values are in the range of 0-255:
>  {{{
>#spearfish
>r.out.gdal in=fields out=fields.tif type=Byte
>qiv fields.tif   # qiv is a quick image viewer
>  }}}
> 
>  but not if all values are out of range:
>  {{{
>r.info -r elevation.dem
>  min=1066
>  max=1840
>r.info -t elevation.dem
>  datatype=CELL
>r.out.gdal in=elevation.dem out=elv.tif type=Byte
> 
>gdalinfo -stats elv.tif | grep STATISTIC
>  STATISTICS_MINIMUM=255
>  STATISTICS_MAXIMUM=255
>  STATISTICS_MEAN=255
>  STATISTICS_STDDEV=0
> 
>r.in.gdal in=elv.tif out=elv2
>r.info -r elv2
>  min=255
>  max=255
>  }}}
> 
> 
>  we can try with half the raster in range:
>  {{{
>r.mapcalc "elev_0 = elevation.dem - 1066"
>r.info -r elev_0
>  min=0
>  max=774
>r.out.gdal in=elev_0 out=elv0.tif type=Byte
>gdalinfo -stats elv0.tif | grep STATISTIC
>  STATISTICS_MINIMUM=0
>  STATISTICS_MAXIMUM=254
>  STATISTICS_MEAN=138.68943338231
>  STATISTICS_STDDEV=56.526296226099
>  }}}
> 
>  If I load elv0.tif into an image viewer it comes out as all black. But in
>  Gimp if you use the eyedropper you can see the index number changes, but
>  the palette is black for all 256 index entries.
> 
>  gdalinfo confirms palette info was not written to file:

I have noticed that r.out.gdal only generates the palette if
G_read_colors() returns 1, i.e. if the map has its own colour table. 
If G_read_colors() falls back to a default rainbow colour table, you
don't get a palette.

I suggest:

--- raster/r.out.gdal/main.c(revision 30389)
+++ raster/r.out.gdal/main.c(working copy)
@@ -125,7 +125,7 @@
 GDALSetRasterColorInterpretation(hBand, GPI_RGB);
 CPLPopErrorHandler();
 
-if( G_read_colors(name, mapset, &sGrassColors ) == 1 )
+if( G_read_colors(name, mapset, &sGrassColors ) >= 0 )
 {
int maxcolor, i;
CELL min, max;

G_read_colors() returns -1 on error (e.g. the map doesn't exist), 0 if
a default rainbow colour table was generated, and 1 if the map has its
own colour table.

AFAICT, the UInt16 case actually works, insofar as the debug output
with DEBUG=3 and the output from gdalinfo look just like they do for
an 8-bit TIFF which works fine in various viewers.

I wouldn't rule out the possibility that popular image viewers can't
handle 16-bit indexed-colour images. 

TIFF's strong-point is that it supports so many different formats. 
It's weak point is also that it supports so many different formats. I
doubt that there is a program in existence which can correctly handle
every possible valid TIFF file.

-- 
Glynn Clements <[EMAIL PROTECTED]>
___
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev