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

[Glynn Clements]

Markus Neteler 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.
> >
> > 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);
> >
> > 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.
> 
> That sounds quite ugly. But from looking at the code: is a major
> rewrite needed to fix that? Since we "just" write out classes?

For now, I have just disabled the test:

	   if(0 && img[row][col] == 0)
		G_set_c_null_value(&files->outbuf[col], 1);

so it will never write out nulls. Previously, these would have been
relatively uncommon anyhow.

I initially suspected that the test should have been against zero, but
in retrospect, it may have been completely absent (i.e. the change was
made on the assumption that zero represented null when it actually
represented zero).

Ultimatly, you would need to look at a pre-5.x version (or to actually
understand the code) in order to determine what it should be doing.

> >> 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.
> 
> ... without an update to i.gensigset I cannot test the FP capabilities of
> i.smap. It seems that it was cloned from i.gensig.

In retrospect, it appears that it's only the training map which is
inherently restricted to integers. E.g. i.gensigset reads the group
maps as CELL then immediately promotes the values to "double".

Unless there's something else I have overlooked, I should have an FP
version ready shortly.

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