[GRASS-dev] Re: [GRASS-user] low radiance values after i.atcor and i.topo.corr

[Glynn Clements]

Dylan Beaudette wrote:

> > >> > And i.smap is gibberish; e.g. write_img() calls G_is_c_null_value() on
> > >> > "char"s, so that module probably hasn't worked since 4.x.
> > >>
> > >> i.smap works, I have tested it recently and also others use it
> > >> regularly.
> 
> This is very troubling, as I have been using i.smap for sometime now 
> (GRASS64-svn), and the output has thus far been convincing. However, I have 
> only been using it with CELL input maps...

On the bright side, this issue may actually never occur in practice. 
Do any of your result maps contain null cells? If I'm correct in
assuming that i.smap should never have created nulls, and you don't
have them, then it doesn't really matter.

> 
> > > Oh, it may produce output, but not necessarily correct output.
> > >
> > > Specifically, write_img() does:
> > >
> > >           if(G_is_c_null_value((CELL *)&img[row][col]))
> > >                   G_set_c_null_value(&files->cellbuf[col], 1);
> 
> Just words, but this again points to the dire need for a comprehensive test 
> suite. 

A test suite wouldn't necessarily catch it. If the bytes in the "img"
array are distributed equally, it would only occur once every 2^32
cells.

> > > But "img" is a "unsigned char **", i.e. a 2D array of bytes. The test
> > > will only succeed if the array contains the sequence 00,00,00,80 (for
> > > little-endian systems) or 80,00,00,00 (big-endian systems). I suspect
> > > that the test was originally against zero, but it was incorrectly
> > > changed when null support was added.
> > >
> > > If that's the case, the net result will be that it writes garbage
> > > where it should be writing nulls, and occasionally writes nulls where
> > > it shouldn't.
> 
> Is there any way to evaluate this? Would this bug only affect input maps that 
> are FP?

Well, I don't understand the algorithm at all. You could try an
example run under gdb, with a breakpoint in write_img(), and look at
what are typical values for the img array. If the values tend to be
small, the chances are that the 0x80 may never occur in practice.

> > That sounds quite ugly. But from looking at the code: is a major
> > rewrite needed to fix that? Since we "just" write out classes?
> >
> > > Beyond that, it seems fairly straightforward. I have committed an
> > > update which compiles without warning, but hasn't been tested.
> >
> > Backported. But...
> >
> > >> Any opinions on
> > >> i.gensig
> > >> i.gensiset
> > >> ? Both are used for supervised classification where you have training
> > >> areas instead of just looking at the pixels as does i.cluster.
> > >
> > > Looking briefly at i.gensig, the algorithm seems to be quite heavily
> > > based upon categories (e.g. using the Cell_stats stuff), so it would
> > > need to be substantially re-written to support FP in any meaningful
> > > sense.
> 
> I have never been able to use the GRASS classification routines on FP maps- 
> usually just results in a map of NULL data or 0.

The modules in question have always read input maps as integers. If
you try using them with FP maps where the values are small (e.g. 
between 0 and 1), then the map will look as if it contains only
zeroes.

-- 
Glynn Clements <glynn at gclements.plus.com>