Hi,
while using r.neighbors on reclassed maps (from r.reclass)
I got some strange results. Is it possible that we have
to added a test into r.neighbors? Or is it a local problem
only?
I would be glad if someone can try r.neighborson a reclassed map
and report.
Thanks,
Markus
Markus Neteler wrote:
while using r.neighbors on reclassed maps (from r.reclass)
I got some strange results.
Are you referring to the result map being almost entirely null?
E.g.
r.neighbors -z input=soils.Kfactor output=foo method=minimum size=5
produces a map which is null except for the very left-hand edge, while
r.mapcalc 'sk=soils.Kfactor'
r.neighbors -z input=sk output=foo method=minimum size=5
seems to work OK.
Is it possible that we have
to added a test into r.neighbors? Or is it a local problem
only?
The bug is in libgis.
The call to G_get_d_raster_row() in readcell.c returns only NaN for
the original (reclass) "soils.Kfactor" map, but sensible values for
the (non-reclass) "sk" map.
I've tracked this down to the double use of a static buffer
(G__.mask_buf) in G_get_raster_row().
For reclass maps, G_get_raster_row() uses G__.mask_buf as a temporary
buffer. It calls G_get_c_raster_row(G__.mask_buf), which in turn calls
embed_nulls(G__.mask_buf), which calls G_get_null_value_row().
However, G_get_null_value_row uses G__.mask_buf itself (via the macro
MASK_BUF) to hold the mask row.
Summary: static "work buffers" are a bad idea.
Unfortunately, some systems don't support alloca(), and some of those
which do don't make it straightforward to use. autoconf has a
solution, but it's messy. And using malloc()/free() for short-lived
buffers could significantly harm performance.
For the time being, I guess that we'll have to just add another static
work buffer for the purpose of reclass maps. However, I'm not yet sure
of the mechanism used for allocating and reallocating such buffers.
--
Glynn Clements <glynn.clements@virgin.net>
On Wed, Jan 02, 2002 at 09:35:28PM +0000, Glynn Clements wrote:
Markus Neteler wrote:
> while using r.neighbors on reclassed maps (from r.reclass)
> I got some strange results.Are you referring to the result map being almost entirely null?
E.g.
r.neighbors -z input=soils.Kfactor output=foo method=minimum size=5
produces a map which is null except for the very left-hand edge, while
r.mapcalc 'sk=soils.Kfactor'
r.neighbors -z input=sk output=foo method=minimum size=5seems to work OK.
> Is it possible that we have
> to added a test into r.neighbors? Or is it a local problem
> only?The bug is in libgis.
The call to G_get_d_raster_row() in readcell.c returns only NaN for
the original (reclass) "soils.Kfactor" map, but sensible values for
the (non-reclass) "sk" map.I've tracked this down to the double use of a static buffer
(G__.mask_buf) in G_get_raster_row().For reclass maps, G_get_raster_row() uses G__.mask_buf as a temporary
buffer. It calls G_get_c_raster_row(G__.mask_buf), which in turn calls
embed_nulls(G__.mask_buf), which calls G_get_null_value_row().
However, G_get_null_value_row uses G__.mask_buf itself (via the macro
MASK_BUF) to hold the mask row.Summary: static "work buffers" are a bad idea.
Unfortunately, some systems don't support alloca(), and some of those
which do don't make it straightforward to use. autoconf has a
solution, but it's messy. And using malloc()/free() for short-lived
buffers could significantly harm performance.For the time being, I guess that we'll have to just add another static
work buffer for the purpose of reclass maps. However, I'm not yet sure
of the mechanism used for allocating and reallocating such buffers.
Glynn,
thanks for looking into this! Do you see a chance to work
around? I have seen some malloc tests in other "configures"
such as GDAL etc. Perhaps they have a functional autoconf
implementation?
Markus
Markus Neteler wrote:
thanks for looking into this! Do you see a chance to work
around?
Sure; I just need to determine how G__.work_buf, G__.mask_buf etc are
managed, and add another one.
I have seen some malloc tests in other "configures"
such as GDAL etc. Perhaps they have a functional autoconf
implementation?
The issue is with alloca(). alloca() suffers from lots of
platform-specific quirks, due the fact that it has to be implemented
by the compiler.
Getting autoconf to detect its presence or absence (and implementing a
workaround if absent) is simple. It's the usage which is messy (see
the documentation of AC_FUNC_ALLOCA in the autoconf Info file for
details).
This is unfortunate, as alloca() is very useful for short-lived
variable-sized buffers. Whereas alloca() may take only a single CPU
clock cycle, malloc() and free() can take an arbitrary amount of time.
An additional malloc/free per call to G_get_raster_row() could result
in a significant performance hit.
--
Glynn Clements <glynn.clements@virgin.net>
Glynn Clements wrote:
I've tracked this down to the double use of a static buffer
(G__.mask_buf) in G_get_raster_row().
That was a problem, but it turns out not to have been the only one.
Both G_get_raster_row() and G_get_raster_row_nomask() had the
following:
for(i=0; i<WINDOW.cols; i++)
G_set_raster_value_c(rast, G__.mask_buf[i], data_type);
This explains why only the first cell in the row was non-null. I have
changed it to:
int size = G_raster_size(data_type);
...
for(i=0; i<WINDOW.cols; i++)
{
G_set_raster_value_c(rast, G__.temp_buf[i], data_type);
rast = G_incr_void_ptr(rast, size);
}
G_get_[fd]_raster_row[_nomask] should now work for reclass maps.
BTW, you need to rebuild everything if you update, as the layout of
"struct G__" has changed.
--
Glynn Clements <glynn.clements@virgin.net>
On Fri, 4 Jan 2002 20:34:30 +0000, Glynn Clements <glynn.clements@virgin.net> wrote:
Markus Neteler wrote:
> thanks for looking into this! Do you see a chance to work
> around?Sure; I just need to determine how G__.work_buf, G__.mask_buf etc are
managed, and add another one.> I have seen some malloc tests in other "configures"
> such as GDAL etc. Perhaps they have a functional autoconf
> implementation?The issue is with alloca(). alloca() suffers from lots of
platform-specific quirks, due the fact that it has to be implemented
by the compiler.Getting autoconf to detect its presence or absence (and implementing a
workaround if absent) is simple. It's the usage which is messy (see
the documentation of AC_FUNC_ALLOCA in the autoconf Info file for
details).This is unfortunate, as alloca() is very useful for short-lived
variable-sized buffers. Whereas alloca() may take only a single CPU
clock cycle, malloc() and free() can take an arbitrary amount of time.
An additional malloc/free per call to G_get_raster_row() could result
in a significant performance hit.
C99 allows variable length automatic arrays. Too bad we probably
can't rely on that feature yet (though, it is supported by gcc-3.0).
int foo (int size)
{
char buff[size];
....
}
--
Eric G. Miller <egm2@jps.net>
Eric G. Miller wrote:
C99 allows variable length automatic arrays. Too bad we probably
can't rely on that feature yet
I doubt that it will be sensible to rely upon C99 features for another
decades or two. It's only within the past few years that the Open
Group decided that it's OK to require an ANSI C compiler for the X11
code.
(though, it is supported by gcc-3.0).
gcc has had variable length arrays for some time. But if you're going
to require gcc, you can count on alloca() being available.
And *most* platforms do actually have alloca(); it's more the fact
that you might have to jump through some hoops to get it (e.g. using
"#pragma alloca" before including any headers), or you have to watch
what compiler/linker options you use.
--
Glynn Clements <glynn.clements@virgin.net>
Hi Glynn and others,
due to the recent fix to the reclass function (thanks!) I have
a related question: There are some raster functions which
currently omit to work on reclass maps. Can we change this now?
Markus
Markus Neteler wrote:
due to the recent fix to the reclass function (thanks!) I have
a related question: There are some raster functions which
currently omit to work on reclass maps. Can we change this now?
Which functions don't work?
--
Glynn Clements <glynn.clements@virgin.net>
On Mon, Jan 07, 2002 at 11:58:11AM +0000, Glynn Clements wrote:
Markus Neteler wrote:
> due to the recent fix to the reclass function (thanks!) I have
> a related question: There are some raster functions which
> currently omit to work on reclass maps. Can we change this now?Which functions don't work?
Those are in question (there are a few more with a check for
G_is_reclass() where it is needed):
./src/mapdev/v.reclass/inter/makemask.c
./src.contrib/NPS/r.out.elas/main.c
./src/raster/r.clump/cmd/main.c
Perhaps the latter is o.k., with above I am not sure.
Markus
Markus Neteler wrote:
> > due to the recent fix to the reclass function (thanks!) I have
> > a related question: There are some raster functions which
> > currently omit to work on reclass maps. Can we change this now?
>
> Which functions don't work?Those are in question (there are a few more with a check for
G_is_reclass() where it is needed):
./src/mapdev/v.reclass/inter/makemask.c
./src.contrib/NPS/r.out.elas/main.c
./src/raster/r.clump/cmd/main.c
These aren't what I'd call "raster functions".
In each case, it may suffice to simply remove the reclass checks. It
would be simpler for someone who is familar with the program to check
it.
--
Glynn Clements <glynn.clements@virgin.net>