Opened 17 months ago

Closed 17 months ago

Last modified 12 months ago

#3857 closed defect (wontfix)

r.mapcalc unable to rename null and cell files

Reported by: Hygsson Owned by: grass-dev@…
Priority: major Milestone: 7.6.2
Component: Raster Version: unspecified
Keywords: r.mapcalc Cc:
CPU: x86-64 Platform: MSWindows 7

Description

Note: This works on Ubuntu without any problems.

While using GRASS GIS 7.6.1 on 64-bit Windows 7, I encountered a problem with r.mapcalc. For a simple stochastic simulation of watershed, I used a following script (here it is recreated for Spearfish dataset).

g.region raster=elevation.dem@PERMANENT
r.surf.random --overwrite output=frequency min=0 max=0
for (( n=1 ; n<21 ; n=$n+1 ))
do
	r.surf.random --overwrite output=errors min=-15 max=15
	r.mapcalc "terrain = elevation.dem@PERMANENT + errors@PERMANENT" --overwrite
	r.watershed --overwrite elevation=terrain@PERMANENT accumulation=accum$n
	r.reclass --overwrite input=accum$n@PERMANENT output=flow$n rules=C:/Users/Hygsson/Desktop/test/reclass.rules
	r.mapcalc "frequency = flow$n + frequency" --overwrite
done

reclass.rules file has a following content:

-500 thru 654 = 0
* = 1

I tried to do it step by step without using the script. The problem arises with this step:

r.mapcalc "frequency = flow$n + frequency" --overwrite

I get this output for each iteration of the loop. Only numbers behind 'unknown/' are different.

WARNING: Unable to rename null file
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\.tmp/unknown\22368.1'
            to
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\cell_misc\frequency\nullcmpr':
            File exists
WARNING: Unable to rename cell file
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\.tmp/unknown\22368.0'
            to
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\fcell\frequency':
            File exists

The desired 'frequency' raster is created, but obviously in a wrong manner, having only one value.

Change History (6)

comment:1 Changed 17 months ago by wenzeslaus

Overwriting map you are reading from is not allowed according to the documentation:

A map cannot be used both as an input and as an output as in this invalid expression oldmap = oldmap + 1, instead a subsequent rename using g.rename is needed when the same name is desired:

r.mapcalc "newmap = oldmap + 1"
g.rename raster=newmap,oldmap

https://grass.osgeo.org/grass76/manuals/r.mapcalc.html#using-the-same-map-for-input-and-output-results

It may work in some cases, but it is not guaranteed. Maybe a check of the inputs and outputs and an error message on Windows and a warning on other platforms would be backward-compatible solution. Would you mind opening a new ticket? On the other hand, having this feature (a = a + 1) seems reasonable (but supporting it is more complex).

comment:2 in reply to:  description ; Changed 17 months ago by hellik

Replying to Hygsson:

Note: This works on Ubuntu without any problems.

While using GRASS GIS 7.6.1 on 64-bit Windows 7, I encountered a problem with r.mapcalc. For a simple stochastic simulation of watershed, I used a following script (here it is recreated for Spearfish dataset).

g.region raster=elevation.dem@PERMANENT
r.surf.random --overwrite output=frequency min=0 max=0
for (( n=1 ; n<21 ; n=$n+1 ))
do
	r.surf.random --overwrite output=errors min=-15 max=15
	r.mapcalc "terrain = elevation.dem@PERMANENT + errors@PERMANENT" --overwrite
	r.watershed --overwrite elevation=terrain@PERMANENT accumulation=accum$n
	r.reclass --overwrite input=accum$n@PERMANENT output=flow$n rules=C:/Users/Hygsson/Desktop/test/reclass.rules
	r.mapcalc "frequency = flow$n + frequency" --overwrite
done

reclass.rules file has a following content:

-500 thru 654 = 0
* = 1

I tried to do it step by step without using the script. The problem arises with this step:

r.mapcalc "frequency = flow$n + frequency" --overwrite

I get this output for each iteration of the loop. Only numbers behind 'unknown/' are different.

WARNING: Unable to rename null file
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\.tmp/unknown\22368.1'
            to
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\cell_misc\frequency\nullcmpr':
            File exists
WARNING: Unable to rename cell file
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\.tmp/unknown\22368.0'
            to
            'C:\Users\Hygsson\Documents\grassdata\Spearfish60_grass7\PERMANENT\fcell\frequency':
            File exists

The desired 'frequency' raster is created, but obviously in a wrong manner, having only one value.

r.mapcalc "frequency = flow$n + frequency" --overwrite

it is an operating system issue/feature/limitation of MS windows, that you are not able to read and write the same file at the same time.

therefore it's not a winGRASS bug.

as Vaclav already mentioned, it may work in linux, but it's not garanteed that it always work in every linux.

so I tend to a won't fix of the ticket.

comment:3 Changed 17 months ago by Hygsson

Resolution: wontfix
Status: newclosed

OK, thank you for your replies. It's good to know that the issue is on Windows' side. I am therefore closing this ticket and will open a new one regarding that input/output check suggested by Wenzeslaus.

comment:4 in reply to:  2 Changed 17 months ago by glynn

Replying to hellik:

it is an operating system issue/feature/limitation of MS windows, that you are not able to read and write the same file at the same time.

By itself, that wouldn't be an issue. GRASS raster output doesn't modify files in-place, but creates a temporary file which is renamed into place by Rast_close(). But r.mapcalc doesn't close the input maps, so their cell/fcell/null files are still open when the output maps are closed. A fix might be as simple as calling close_maps() from execute(), before the output maps are closed.

The temporary file is used for cell, fcell and null files. Other files are modified in-place, but these are normally either closed before raster I/O starts or opened after raster I/O has completed.

Aside: this is why G_tempfile() creates files in the mapset, not /tmp or $TMPDIR; they have to be on the same filesystem as the mapset so that rename() works.

comment:5 Changed 12 months ago by Nikos Alexandris

I read this thread again and I am still not happy with the "it may work in some cases" statement. In which cases will it work and in which not? Can someone help to dig out the "documented" behavior?

Glynn's suggested fix is applied here: https://github.com/OSGeo/grass/blob/master/raster/r.mapcalc/evaluate.c#L395.

comment:6 in reply to:  5 Changed 12 months ago by glynn

Replying to Nikos Alexandris:

I read this thread again and I am still not happy with the "it may work in some cases" statement. In which cases will it work and in which not? Can someone help to dig out the "documented" behavior?

With that fix, it should work so long as the maps are actual GRASS maps, not external files linked via r.external(.out).

Things which might still be an issue:

  • Overwriting a map doesn't systematically delete files belonging to the old map. Look at close_new to see which files are removed or replaced. This issue isn't specific to r.mapcalc, or to overwriting outputs with inputs, but with overwriting in general.
  • When copying a map without any intervening calculation (i.e. the entire expression on the right-hand side of a top-level assignment is just a map, optionally with a neighbourhood modifier, but no category/colour modifier), the input map's categories, colours and history are copied to the output map. This won't work if the input map and output map are the same map, and it won't work reliably if the input map is one of the outputs (the result will depend upon the order in which the outputs are closed). Closing an output map will replace all of the standard support files (see close_new as mentioned above), so copying those files will copy the new version if the output map has already been closed. This situation doesn't seem particularly likely to occur in practice.
Note: See TracTickets for help on using tickets.