[GRASS-dev] G7: how to get rid of multiple listing of identical map names?

GRASS GIS GRASS GIS developer
1

Hi Huidae, all,

if I recall correctly you (Huidae) fixed in trunk the problem that
same map names in different mapsets (which are in the search path) are
listed several times.

This still happens for d.rast, r.info (see below) and others. I don't
remember the patch number. Perhaps it could be put at library level
to make all modules better?

Here the simulation:

# clone landsat mapset:
[neteler@oboe nc_spm_08_grass7]$ cp -rp landsat landsat2
[neteler@oboe nc_spm_08_grass7]$ cp -rp landsat landsat3

# run
grass70 ~/grassdata/nc_spm_08_grass7/landsat3/

r.info lsat5_1987_50
WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
WARNING: Using <lsat5_1987_50@landsat3>
WARNING: 'cellhd/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'cellhd/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
WARNING: Using <lsat5_1987_50@landsat3>
WARNING: 'cellhd/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'cellhd/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
WARNING: Using <lsat5_1987_50@landsat3>
WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
WARNING: Using <lsat5_1987_50@landsat3>
WARNING: 'cats/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'cats/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
WARNING: Using <lsat5_1987_50@landsat3>
WARNING: 'hist/lsat5_1987_50' was found in more mapsets (also found in
         <landsat2>)
WARNING: 'hist/lsat5_1987_50' was found in more mapsets (also found in
         <landsat>)
[....]

g.mapsets -p
Accessible mapsets:
landsat3 PERMANENT landsat2 landsat

I don't know where to look for the problem.

thanks for any pointers,
Markus

2

Hi Markus,

That was r60261. It just suppresses the warnings. IMHO, those found in multiple mapsets warnings are very noisy and unnecessary. Sometimes it prints exactly the same messages multiple times and sometimes breaks outputs. I think it’s better to not print the warnings by default and print them based on an argument or something, if we really need them.

Regards,
Huidae

On Jun 21, 2014 9:01 PM, “Markus Neteler” <neteler@osgeo.org> wrote:

Hi Huidae, all,

if I recall correctly you (Huidae) fixed in trunk the problem that
same map names in different mapsets (which are in the search path) are
listed several times.

This still happens for d.rast, r.info (see below) and others. I don’t
remember the patch number. Perhaps it could be put at library level
to make all modules better?

Here the simulation:

clone landsat mapset:

[neteler@oboe nc_spm_08_grass7]$ cp -rp landsat landsat2
[neteler@oboe nc_spm_08_grass7]$ cp -rp landsat landsat3

run

grass70 ~/grassdata/nc_spm_08_grass7/landsat3/

r.info lsat5_1987_50
WARNING: ‘cell/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘cell/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: Using lsat5_1987_50@landsat3
WARNING: ‘cellhd/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘cellhd/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: Using lsat5_1987_50@landsat3
WARNING: ‘cellhd/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘cellhd/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: Using lsat5_1987_50@landsat3
WARNING: ‘cell/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘cell/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: Using lsat5_1987_50@landsat3
WARNING: ‘cats/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘cats/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: Using lsat5_1987_50@landsat3
WARNING: ‘hist/lsat5_1987_50’ was found in more mapsets (also found in
)
WARNING: ‘hist/lsat5_1987_50’ was found in more mapsets (also found in
)
[…]

g.mapsets -p
Accessible mapsets:
landsat3 PERMANENT landsat2 landsat

I don’t know where to look for the problem.

thanks for any pointers,
Markus

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

3

On Sun, Jun 22, 2014 at 5:16 AM, Huidae Cho <grass4u@gmail.com> wrote:

Hi Markus,

That was r60261. It just suppresses the warnings.

I see, so G_find_raster2() calls G_find_file2() calls find_file() in
lib/gis/find_file.c

IMHO, those found in
multiple mapsets warnings are very noisy and unnecessary.

Yes, definitely. And it is a "new" issue in GRASS 7, same use cases
behave properly in GRASS 6.

Sometimes it
prints exactly the same messages multiple times and sometimes breaks
outputs. I think it's better to not print the warnings by default and print
them based on an argument or something, if we really need them.

Should r60261 better be fixed in
lib/gis/find_file.c

?

Best
Markus

4

We have the same “new” issue in 6.4. I would just remove those warnings.

It looks like those warning messages are somehow related to r22844 by Glynn. The messages may be useful, but printing one warning per element for the same map is too much and the user doesn’t need to know what element is being accessed. If we definitely want to show these similar messages, they should be per map, not per element per map to avoid too much and annoying verbosity.

But still, IMHO, I don’t see them that useful because the find_file returns the first file found in the search path based on a known and defined rule, which the user should already be familiar with. It’s not that something unexpected is happening. Why not changing them to G_verbose_message or even G_debug?

···

On Sun, Jun 22, 2014 at 3:25 AM, Markus Neteler <neteler@osgeo.org> wrote:

On Sun, Jun 22, 2014 at 5:16 AM, Huidae Cho <grass4u@gmail.com> wrote:

Hi Markus,

That was r60261. It just suppresses the warnings.

I see, so G_find_raster2() calls G_find_file2() calls find_file() in
lib/gis/find_file.c

IMHO, those found in
multiple mapsets warnings are very noisy and unnecessary.

Yes, definitely. And it is a “new” issue in GRASS 7, same use cases
behave properly in GRASS 6.

Sometimes it
prints exactly the same messages multiple times and sometimes breaks
outputs. I think it’s better to not print the warnings by default and print
them based on an argument or something, if we really need them.

Should r60261 better be fixed in
lib/gis/find_file.c

?

Best
Markus

5

The exact same issue was discussed years ago (http://osgeo-org.1560.x6.nabble.com/gis-m-crashes-on-zoom-to-map-existing-in-more-than-one-mapset-td4021119.html), but was never fixed. I think this warning is more of a reminder, so requiring --verbose to see this message doesn’t seem to make much sense although personally I hate to see the warning in module outputs because I know what I’m doing. Especially, in r.info the message breaks the output format.

···

Anyway, if we want to keep the message, I think it’s reasonable to change it to G_verbose_message even if then it cannot serve as a reminder.

On Sun, Jun 22, 2014 at 10:06 AM, Vaclav Petras <wenzeslaus@gmail.com> wrote:

Verbose message per map sounds good.

On Jun 22, 2014 8:36 AM, “Huidae Cho” <grass4u@gmail.com> wrote:

We have the same “new” issue in 6.4. I would just remove those warnings.

It looks like those warning messages are somehow related to r22844 by Glynn. The messages may be useful, but printing one warning per element for the same map is too much and the user doesn’t need to know what element is being accessed. If we definitely want to show these similar messages, they should be per map, not per element per map to avoid too much and annoying verbosity.

But still, IMHO, I don’t see them that useful because the find_file returns the first file found in the search path based on a known and defined rule, which the user should already be familiar with. It’s not that something unexpected is happening. Why not changing them to G_verbose_message or even G_debug?

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

On Sun, Jun 22, 2014 at 3:25 AM, Markus Neteler <neteler@osgeo.org> wrote:

On Sun, Jun 22, 2014 at 5:16 AM, Huidae Cho <grass4u@gmail.com> wrote:

Hi Markus,

That was r60261. It just suppresses the warnings.

I see, so G_find_raster2() calls G_find_file2() calls find_file() in
lib/gis/find_file.c

IMHO, those found in
multiple mapsets warnings are very noisy and unnecessary.

Yes, definitely. And it is a “new” issue in GRASS 7, same use cases
behave properly in GRASS 6.

Sometimes it
prints exactly the same messages multiple times and sometimes breaks
outputs. I think it’s better to not print the warnings by default and print
them based on an argument or something, if we really need them.

Should r60261 better be fixed in
lib/gis/find_file.c

?

Best
Markus

6

Huidae Cho wrote:

We have the same "new" issue in 6.4. I would just remove those warnings.

It looks like those warning messages are somehow related to r22844 by
Glynn.

The only consequence of r22844 is that if a file is found in multiple
mapsets, the warnings identify all of the mapsets in which it was
found, not just the one being used.

The old version would print

  WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in landsat3).

[The use of "also" is misleading, as that's the version which is
actually used.]

The new version prints

  WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in landsat2).
  WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also found in landsat).
  WARNING: Using <lsat5_1987_50@landsat3>

The issue of the warning(s) being printed per-element rather than
per-map is a consequence of the check being performed in the library
rather than in the module. It applies equally to both the old and new
versions.

The messages may be useful, but printing one warning per element for
the same map is too much and the user doesn't need to know what element is
being accessed. If we definitely want to show these similar messages, they
should be per map, not per element per map to avoid too much and annoying
verbosity.

It would be possible to avoid the per-element warnings by using
G_suppress_warnings() in the module to suppress warnings on all
elements after the first. However, that could suppress other warnings.

Or we could add something to lib/gis/find_file.c to control which
elements are eligible for these warnings (e.g. for a raster map, only
warning about the cellhd element should be sufficient).

Or we could keep track of the last such warning generated, and
suppress the warning if the map and mapset match those of the previous
warning.

But still, IMHO, I don't see them *that* useful because the find_file
returns the first file found in the search path based on a known and
defined rule, which the user should already be familiar with.

People aren't computers. They don't necessarily know the current
mapset search path and/or the contents of those mapsets at all times.
And even if they do, they may not always draw the relevant conclusions
from that information.

Without the warnings, it's entirely conceivable that a user could end
up unknowingly getting bogus results because a map which they intended
to use was "shadowed" by a map in another mapset.

And the rules aren't actually all that well defined. We've even had
developers bitten by oversights regarding the search path, i.e.
assuming that a map which is known (or assumed) to exist in the
current mapset can be reliably accessed using an unqualified name.

This wasn't always the case, as the current mapset wasn't guaranteed
to be the first mapset in the search path. In the end, the code was
changed to force this to be the case, because expecting developers not
to make this mistake was considered unrealistic.

One consequence of this change is that the current mapset search path
isn't necessarily what the user set with "g.mapsets set=..."; the
current mapset will always be at the front of the list.

Another consequence is that changing the current mapset with g.mapset
will implicitly change the search path (the previous current mapset
will either revert to its specified position in the list, or be
removed from the search path, depending upon whether it was explicitly
part of the search path).

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

7

On Mon, Jun 23, 2014 at 6:54 AM, Glynn Clements <glynn@gclements.plus.com>
wrote:

Huidae Cho wrote:

> We have the same "new" issue in 6.4. I would just remove those warnings.
>
> It looks like those warning messages are somehow related to r22844 by
> Glynn.

The only consequence of r22844 is that if a file is found in multiple
mapsets, the warnings identify all of the mapsets in which it was
found, not just the one being used.

The old version would print

        WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also
found in landsat3).

[The use of "also" is misleading, as that's the version which is
actually used.]

The new version prints

        WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also
found in landsat2).
        WARNING: 'cell/lsat5_1987_50' was found in more mapsets (also
found in landsat).
        WARNING: Using <lsat5_1987_50@landsat3>

The issue of the warning(s) being printed per-element rather than
per-map is a consequence of the check being performed in the library
rather than in the module. It applies equally to both the old and new
versions.

> The messages may be useful, but printing one warning per element for
> the same map is too much and the user doesn't need to know what element
is
> being accessed. If we definitely want to show these similar messages,
they
> should be per map, not per element per map to avoid too much and annoying
> verbosity.

It would be possible to avoid the per-element warnings by using
G_suppress_warnings() in the module to suppress warnings on all
elements after the first. However, that could suppress other warnings.

I'm against this idea for that reason and because we have to change modules.

Or we could add something to lib/gis/find_file.c to control which
elements are eligible for these warnings (e.g. for a raster map, only
warning about the cellhd element should be sufficient).

We could pick one representative element, which must be open first (cellhd
for raster, head for vector, ... for others?) and print only mapset names
per map. However, if the search path changes between find_file calls and we
print only the first warning for the same map, we can miss some mapsets
added later or, in the worst case, it may not print an important "reminder"
when really needed.

Or we could keep track of the last such warning generated, and
suppress the warning if the map and mapset match those of the previous
warning.

Not just the last such warning because we never know when the same map will
be accessed again from a module. E.g., cellhd/raster1, cellhd/raster2,
cell/raster1, cell/raster2.

In any case, we can take advantage of further aggregating the warning into
one each time (either per-element or per-map):

WARNING: <lsat5_1987_50> raster map was also found in <landsat2> and
<landsat>. Using <lsat5_1987_50@landsat3>.

> But still, IMHO, I don't see them *that* useful because the find_file
> returns the first file found in the search path based on a known and
> defined rule, which the user should already be familiar with.

People aren't computers. They don't necessarily know the current
mapset search path and/or the contents of those mapsets at all times.
And even if they do, they may not always draw the relevant conclusions
from that information.

Without the warnings, it's entirely conceivable that a user could end
up unknowingly getting bogus results because a map which they intended
to use was "shadowed" by a map in another mapset.

And the rules aren't actually all that well defined. We've even had
developers bitten by oversights regarding the search path, i.e.
assuming that a map which is known (or assumed) to exist in the
current mapset can be reliably accessed using an unqualified name.

This wasn't always the case, as the current mapset wasn't guaranteed
to be the first mapset in the search path. In the end, the code was
changed to force this to be the case, because expecting developers not
to make this mistake was considered unrealistic.

One consequence of this change is that the current mapset search path
isn't necessarily what the user set with "g.mapsets set=..."; the
current mapset will always be at the front of the list.

Another consequence is that changing the current mapset with g.mapset
will implicitly change the search path (the previous current mapset
will either revert to its specified position in the list, or be
removed from the search path, depending upon whether it was explicitly
part of the search path).

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

8

Huidae Cho wrote:

> It would be possible to avoid the per-element warnings by using
> G_suppress_warnings() in the module to suppress warnings on all
> elements after the first. However, that could suppress other warnings.

I'm against this idea for that reason and because we have to change modules.

Another option which I overlooked (but which has the same issue) is to
perform a single G_find_* call for a representative element, then pass
either an explicit mapset or a fully-qualified name in subsequent
calls.

In fact, r.info (which was the original example) already queries the
mapset, but only uses it for display.

This is a change from 6.x, which uses the found mapset when opening
support files (cats, history, etc). That was probably done when I
fixed the various library functions to consistently support the use of
fully-qualified names. The issue of multiple warnings (and a more
fundamental issue discussed below) didn't come up at that time.

Between the two, using a fully-qualified name would be preferable.
Part of the reason for the original change was to eliminate the need
for modules to deal with mapsets explicitly. Functions which accept a
mapset argument should normally have "" as the mapset. The intent was
that the mapset argument could eventually be removed altogether.

> Or we could keep track of the last such warning generated, and
> suppress the warning if the map and mapset match those of the previous
> warning.

Not just the last such warning because we never know when the same map will
be accessed again from a module. E.g., cellhd/raster1, cellhd/raster2,
cell/raster1, cell/raster2.

The problem with that is that the memory requirements will grow over
time. And persistent applications such as QGIS will probably need some
way to reset the list.

OTOH, the "last warning" approach probably isn't good enough; modules
which process multiple maps often perform several iterations over the
list of maps: e.g. one for the cellhd for each map, one to open each
map, one to read the support files for each map.

But there's a rather more fundamental issue here: if G_find_file() is
called for an element which isn't guaranteed to exist for every map,
it will "find" the first map which actually contains that element,
which isn't necessarily the same map as for the other elements.

This doesn't affect the primary components of a map, because
Rast_open_old() first determines the mapset, then passes an explicit
mapset to the various G_open_* functions.

So, I think that G_find_file() etc should only be used in "find mode"
(with an empty mapset) for the "primary" element of a map (i.e.
"cellhd" for raster maps, the map directory for vector maps). Other
components should be located by first finding the mapset, then using
the mapset to locate any additional components.

The old approach of having each module perform a G_find_file() to
determine the mapset avoided this issue, but placed the burden of
implementing this logic onto the individual modules (and some didn't,
meaning that they didn't handle fully-qualified names correctly).

A key issue is that G_find_file() is used for two distinct purposes:
one is finding maps (either by deconstructing fully-qualfied names or
by scanning the mapset search path), the other is generating the
filenames for the individual elements of a map.

These should probably be different functions (or rather, sets of
functions). But that may be too invasive (e.g. G_open_* etc implicitly
use G_find_file). A workaround would be to maintain a mapping which
connects an element name to a primary element, and always perform any
search for the primary element.

We might also want to avoid scanning the mapset search path repeatedly
for performance reasons, and maintaining a cache of found maps. If
that's done, the issue with multiple warnings would become moot.
However, this also raises issues with regard to persistent
applications.

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

9

Glynn Clements wrote:

But there's a rather more fundamental issue here: if G_find_file() is
called for an element which isn't guaranteed to exist for every map,
it will "find" the first map which actually contains that element,
which isn't necessarily the same map as for the other elements.

This doesn't affect the primary components of a map, because
Rast_open_old() first determines the mapset, then passes an explicit
mapset to the various G_open_* functions.

So, I think that G_find_file() etc should only be used in "find mode"
(with an empty mapset) for the "primary" element of a map (i.e.
"cellhd" for raster maps, the map directory for vector maps). Other
components should be located by first finding the mapset, then using
the mapset to locate any additional components.

Fixed (hopefully) in r61840.

This change also reduces the number of warnings; a warning is only
generated when searching for the primary element.

Given that it's a fairly fundamental change, it should be tested.

OTOH, given that it fixes a fairly fundamental bug, it should go into
at least one beta prior to any final release.

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

10

On Mon, Sep 8, 2014 at 6:06 PM, Glynn Clements <glynn@gclements.plus.com>
wrote:

Glynn Clements wrote:

> But there's a rather more fundamental issue here: if G_find_file() is
> called for an element which isn't guaranteed to exist for every map,
> it will "find" the first map which actually contains that element,
> which isn't necessarily the same map as for the other elements.
>
> This doesn't affect the primary components of a map, because
> Rast_open_old() first determines the mapset, then passes an explicit
> mapset to the various G_open_* functions.
>
> So, I think that G_find_file() etc should only be used in "find mode"
> (with an empty mapset) for the "primary" element of a map (i.e.
> "cellhd" for raster maps, the map directory for vector maps). Other
> components should be located by first finding the mapset, then using
> the mapset to locate any additional components.

Fixed (hopefully) in r61840.

It seems that r61840 breaks a lot of tests. The yesterdays version (which
is by chance r61839) is working. There are errors like

ERROR: Raster map <elevation> not found

for example for g.region and r.info.

http://fatra.cnr.ncsu.edu/grassgistests/reports_for_date-2014-09-08-07-00/report_for_nc_spm_08_grass7_nc/testfiles.html
http://fatra.cnr.ncsu.edu/grassgistests/reports_for_date-2014-09-09-07-00/report_for_nc_spm_08_grass7_nc/testfiles.html
http://fatra.cnr.ncsu.edu/grassgistests/reports_for_date-2014-09-09-07-00/report_for_nc_spm_08_grass7_nc/raster/r.slope.aspect/test_r_slope_aspect/index.html

This change also reduces the number of warnings; a warning is only
generated when searching for the primary element.

Given that it's a fairly fundamental change, it should be tested.

OTOH, given that it fixes a fairly fundamental bug, it should go into
at least one beta prior to any final release.

--
Glynn Clements <glynn@gclements.plus.com>
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

11

Vaclav Petras wrote:

It seems that r61840 breaks a lot of tests.

No idea how that passed even minimal testing; hopefully fixed in r61853.

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

12

On Sep 10, 2014 2:57 PM, “Glynn Clements” <glynn@gclements.plus.com> wrote:

Vaclav Petras wrote:

It seems that r61840 breaks a lot of tests.

No idea how that passed even minimal testing;

And also not all tests were broken. That’s why we need a lot of them!

hopefully fixed in r61853.

–
Glynn Clements <glynn@gclements.plus.com>

13

On Tue, Sep 9, 2014 at 12:06 AM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Glynn Clements wrote:

But there's a rather more fundamental issue here: if G_find_file() is
called for an element which isn't guaranteed to exist for every map,
it will "find" the first map which actually contains that element,
which isn't necessarily the same map as for the other elements.

...

OTOH, given that it fixes a fairly fundamental bug, it should go into
at least one beta prior to any final release.

So that would be r61840 + r61853 to be backported.
But how to test it in trunk?

Markus

14

Markus Neteler wrote:

> OTOH, given that it fixes a fairly fundamental bug, it should go into
> at least one beta prior to any final release.

So that would be r61840 + r61853 to be backported.
But how to test it in trunk?

Backport to beta, see if any issues show up.

In terms of constructing test cases, you need a location where the
"... was found in more mapsets" warnings are common.

As a rough guide: clone a mapset, place the clone ahead of the
original in the mapset search path, remove various support files from
the cloned maps, then run commands which try to use those support
files, and check that they aren't "finding" the files from the
original maps.

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

15

On Tue, Sep 16, 2014 at 12:41 PM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Neteler wrote:

> OTOH, given that it fixes a fairly fundamental bug, it should go into
> at least one beta prior to any final release.

So that would be r61840 + r61853 to be backported.
But how to test it in trunk?

Backport to beta, see if any issues show up.

Done in r61995 (relbr7).

In terms of constructing test cases, you need a location where the
"... was found in more mapsets" warnings are common.

As a rough guide: clone a mapset, place the clone ahead of the
original in the mapset search path, remove various support files from
the cloned maps, then run commands which try to use those support
files, and check that they aren't "finding" the files from the
original maps.

Thanks - this should be done on various OS.

Markus