[GRASS5] bug in d.what.rast

GRASS GRASS developer
1

Folks,

d.what.rast in GRASS pre2 is broken. The -c flag produces incorrect results.
The problem was introduced somewhere between beta11 and pre2. The beta11
version works. I did track down exactly what the problem was, and it looks
like it's still there in the current version.

Briefly, the definition of the show_utm function was modified to pass the
name of the mapset as an argument. That causes show_utm to skip the call to
G_get_cellhd that fills the cellhd structure; the column and row numbers are
then calculated on the basis of cellhd.north = cellhd.west = 0.

The easiest solution is just to revert the code to the earlier version.

Roger Miller

2

On Sat, Mar 02, 2002 at 07:35:14PM -0700, Roger Miller wrote:

Folks,

d.what.rast in GRASS pre2 is broken. The -c flag produces incorrect
results. The problem was introduced somewhere between beta11 and
pre2. The beta11 version works. I did track down exactly what the
problem was, and it looks like it's still there in the current
version.

Briefly, the definition of the show_utm function was modified to pass
the name of the mapset as an argument. That causes show_utm to skip
the call to G_get_cellhd that fills the cellhd structure; the column
and row numbers are then calculated on the basis of cellhd.north =
cellhd.west = 0.

The easiest solution is just to revert the code to the earlier
version.

The changelog seems to indicate there was a problem before. So,
maybe what's needed is just to move the G_get_cellhd() function
call after the if(!mapset){...}, which seems to be the error..
I'm not sure if there's a good reason for cellhd to be static
either... ??

--
Eric G. Miller <egm2@jps.net>

3

Eric G. Miller wrote:

> d.what.rast in GRASS pre2 is broken. The -c flag produces incorrect
> results. The problem was introduced somewhere between beta11 and
> pre2. The beta11 version works. I did track down exactly what the
> problem was, and it looks like it's still there in the current
> version.
>
> Briefly, the definition of the show_utm function was modified to pass
> the name of the mapset as an argument. That causes show_utm to skip
> the call to G_get_cellhd that fills the cellhd structure; the column
> and row numbers are then calculated on the basis of cellhd.north =
> cellhd.west = 0.
>
> The easiest solution is just to revert the code to the earlier
> version.

The changelog seems to indicate there was a problem before. So,
maybe what's needed is just to move the G_get_cellhd() function
call after the if(!mapset){...}, which seems to be the error..
I'm not sure if there's a good reason for cellhd to be static
either... ??

In the previous version, mapset was always NULL the first time that
show_utm() was called, ensuring that the Cell_head was read.
Thereafter, mapset would be non-NULL, and the existing Cell_head would
be read.

So, the bug was introduced in 1.4, and the fix is to always call
G_get_cellhd(). And there isn't any reason for cellhd to be static
now.

Also, I'm suspicious of Huidae's "fix" in 1.3:

- n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
- e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
+ n_row = (int) ((cellhd.north - north) / window->ns_res);
+ e_col = (int) ((east - cellhd.west) / window->ew_res);

Either of:

  n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
  e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
and:
  n_row = (int) ((window->north - north) / window->ns_res);
  e_col = (int) ((east - window->west) / window->ew_res);

make sense, depending upon whether you want coordinates in map cells
or window cells. But I can't see any obvious significance to the
values returned by the existing code.

Another question springs to mind, namely whether d.what.rast should
read the region from the monitor rather than using the current window,
as the monitor's region reflects what is actually displayed in the
window.

--
Glynn Clements <glynn.clements@virgin.net>

4

On Sun, Mar 03, 2002 at 05:04:01AM +0000, Glynn Clements wrote:

In the previous version, mapset was always NULL the first time that
show_utm() was called, ensuring that the Cell_head was read.
Thereafter, mapset would be non-NULL, and the existing Cell_head would
be read.

So, the bug was introduced in 1.4, and the fix is to always call
G_get_cellhd(). And there isn't any reason for cellhd to be static
now.

This is kind of what I thought, but I didn't look close enough to
make a decision.

Also, I'm suspicious of Huidae's "fix" in 1.3:

- n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
- e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
+ n_row = (int) ((cellhd.north - north) / window->ns_res);
+ e_col = (int) ((east - cellhd.west) / window->ew_res);

Either of:

  n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
  e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
and:
  n_row = (int) ((window->north - north) / window->ns_res);
  e_col = (int) ((east - window->west) / window->ew_res);

make sense, depending upon whether you want coordinates in map cells
or window cells. But I can't see any obvious significance to the
values returned by the existing code.

The man page says it outputs row/col for the entire map region. It's
a bit ambiguous what that means. One reason to use the window vs.
the actual cell region, is probably because the coordinates derived
from a mouse click are based on the display window. It should be
consistent with how the categories are looked up (otherwise, it'd
likely report a row/col value that doesn't match the category reported).

Another question springs to mind, namely whether d.what.rast should
read the region from the monitor rather than using the current window,
as the monitor's region reflects what is actually displayed in the
window.

Probably should use the monitor's. In practice, they'll probably
be the same as user's aren't likely to change the region setting
between drawing the raster and querying it. Still, if they do,
it could lead to interesting results...

--
Eric G. Miller <egm2@jps.net>

5

The man page says it outputs row/col for the entire map region. It's
a bit ambiguous what that means. One reason to use the window vs.
the actual cell region, is probably because the coordinates derived
from a mouse click are based on the display window. It should be
consistent with how the categories are looked up (otherwise, it'd
likely report a row/col value that doesn't match the category reported).

> Another question springs to mind, namely whether d.what.rast should
> read the region from the monitor rather than using the current window,
> as the monitor's region reflects what is actually displayed in the
> window.

Probably should use the monitor's. In practice, they'll probably
be the same as user's aren't likely to change the region setting
between drawing the raster and querying it. Still, if they do,
it could lead to interesting results...

The -c option is intended to give the location in the map, hence it
initiailizes the cellhd structure from the raster file.

Printing the monitor row/col would be a useless result -- at least for my
purpose. The idea is to locate a feature in the map. I'm not sure what
other people use it for, but I use it to locate the row/column location of a
feature in a groundwater model where I need to know row and column. I
imagine that other people might have similar uses for it. I don't think that
GRASS offers any other facility to do the same thing. The location of the
feature on the monitor has no value that I can imagine.

6

Eric G. Miller wrote:

> Also, I'm suspicious of Huidae's "fix" in 1.3:
>
> - n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
> - e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
> + n_row = (int) ((cellhd.north - north) / window->ns_res);
> + e_col = (int) ((east - cellhd.west) / window->ew_res);
>
> Either of:
>
> n_row = (int) ((cellhd.north - north) / cellhd.ns_res);
> e_col = (int) ((east - cellhd.west) / cellhd.ew_res);
> and:
> n_row = (int) ((window->north - north) / window->ns_res);
> e_col = (int) ((east - window->west) / window->ew_res);
>
> make sense, depending upon whether you want coordinates in map cells
> or window cells. But I can't see any obvious significance to the
> values returned by the existing code.

The man page says it outputs row/col for the entire map region. It's
a bit ambiguous what that means. One reason to use the window vs.
the actual cell region, is probably because the coordinates derived
from a mouse click are based on the display window. It should be
consistent with how the categories are looked up (otherwise, it'd
likely report a row/col value that doesn't match the category reported).

The category reported is for the cell which is displayed beneath the
mouse cursor. But that cell can be addressed using either map row/col
or window row/col.

The main reasons that I can see for using window row/col are:

1. Those correspond to r.mapcalc's row() and col() functions, and I
can't immediately think of anywhere else that rows/cols are visible to
the user. However, r.mapcalc's row() and col() functions use one-based
indices, while "d.what.rast -c" uses zero-based indices.

2. When specifying multiple maps, the window row/col are the same for
all of them, whereas the map row/col varies with a map's boundaries
and resolution.

However, if the intention is to use window rows/cols, there isn't any
point in passing the map name or mapset to show_utm(). Those values
are only used to retrieve the map's Cell_head, which is only required
if you want to report map rows/cols.

--
Glynn Clements <glynn.clements@virgin.net>

7

Roger Miller wrote:

> The man page says it outputs row/col for the entire map region. It's
> a bit ambiguous what that means. One reason to use the window vs.
> the actual cell region, is probably because the coordinates derived
> from a mouse click are based on the display window. It should be
> consistent with how the categories are looked up (otherwise, it'd
> likely report a row/col value that doesn't match the category reported).
>
> > Another question springs to mind, namely whether d.what.rast should
> > read the region from the monitor rather than using the current window,
> > as the monitor's region reflects what is actually displayed in the
> > window.
>
> Probably should use the monitor's. In practice, they'll probably
> be the same as user's aren't likely to change the region setting
> between drawing the raster and querying it. Still, if they do,
> it could lead to interesting results...

The -c option is intended to give the location in the map, hence it
initiailizes the cellhd structure from the raster file.

Printing the monitor row/col would be a useless result -- at least for my
purpose. The idea is to locate a feature in the map. I'm not sure what
other people use it for, but I use it to locate the row/column location of a
feature in a groundwater model where I need to know row and column. I
imagine that other people might have similar uses for it. I don't think that
GRASS offers any other facility to do the same thing. The location of the
feature on the monitor has no value that I can imagine.

Note that "window" row/col refers to the current window, in the sense
of G_get_window() and the WIND file, not the monitor window (pixel
coordinates).

--
Glynn Clements <glynn.clements@virgin.net>

8

On Sunday 03 March 2002 08:55, Glynn Clements wrote:

> Printing the monitor row/col would be a useless result -- at least for my
> purpose. The idea is to locate a feature in the map. I'm not sure what
> other people use it for, but I use it to locate the row/column location
> of a feature in a groundwater model where I need to know row and column.
> I imagine that other people might have similar uses for it. I don't
> think that GRASS offers any other facility to do the same thing. The
> location of the feature on the monitor has no value that I can imagine.

Note that "window" row/col refers to the current window, in the sense
of G_get_window() and the WIND file, not the monitor window (pixel
coordinates).

The program needs to print the row and column coordinates from the raster
file, as is supposed to now. The row and column in the current region as
displayed on the monitor and stored in the WIND file are not useful.

Roger Miller