[GRASS-dev] [GRASS GIS] #520: r.terraflow/iostream does not respect STREAM_DIR option

#520: r.terraflow/iostream does not respect STREAM_DIR option
--------------------+-------------------------------------------------------
Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Keywords: | Platform: All
      Cpu: All |
--------------------+-------------------------------------------------------
In r.terraflow, the default directory for temporary files is set to
/var/tmp, contrary to GRASS policy. Further on, specifying a temporary
directory with option STREAM_DIR is ignored, /var/tmp is used anyway.

For the analysis I want to run, r.terraflow wants "at least 25.12G" (quote
from r.terraflow output) of disk space. That much space is not available
on the partition where my /var/tmp is. For such cases I have a separate
hard drive with several 100 GB of free space and tuned for speed. At least
for me, r.terraflow does not work on massive grids as long as it does not
use the given STREAM_DIR.

devbr_6 r36206 Linux 64bit

BTW, r.watershed does the job for me in about 15 minutes, peak memory
consumption is about 7GB during A* Search, after that less.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

I got trapped by the same problem yesterday and suggest to use the .tmp in
the mapset as default. Then it is in the HOME directory which is typically
easier to manage.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

attached patch uses dirname(G_tempfile()) to create tmp dir in mapset
instead of /var/tmp/.

Markus

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by glynn):

Replying to [comment:2 neteler]:
> attached patch uses dirname(G_tempfile()) to create tmp dir in mapset
instead of /var/tmp/.

It should use either the directory specified by the stream_dir= option or
$TMPDIR, so that the user can choose a directory on a partition which has
enough space.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

Fixed patch uploaded, please test/comment.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by hamish):

the bugfix part of it looks good:
{{{
-flowStream = new AMI_STREAM<waterWindowBaseType>("/var/tmp/flowStream");
+flowStream = new AMI_STREAM<waterWindowBaseType>(streamdir->answer);
}}}

For the change of the default placement from /var/tmp/ to $MAPSET/.tmp/, I
don't think using G_tempfile() in the option definition section is allowed
(ie before G_gisinit() has run), as it could end in G_fatal_error() which
requires that. Also the call as written would leave behind an empty
tempfile each time you ran the module, even if just --help or --html-
description.

if it is changed away from /var/tmp/, docs would need updating too, but
I'm not sure if it is a good idea to change the default behaviour in the
stable branch.

It has been left in a state "contrary to GRASS policy" due to the large
file sizes. there was previous discussion on the mailing list about that
some years ago.

tip: if you increase the memory= option to (e.g.) 75% of your system RAM,
the needed tempfile space goes down a lot.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by mmetz):

Replying to [comment:4 neteler]:
> Fixed patch uploaded, please test/comment.

r.terraflow is now using grass tmp dir and works, but still not respecting
STREAM_DIR option.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

New patch uploaded with bugfix only (so back to /var/tmp/ default). The
bug is AFAIK here:

[source:grass/branches/develbranch_6/raster/r.terraflow/main.cc#L179]

The assignment to opt seems to fail:
{{{
   opt->mem = atoi(mem->answer);
   opt->streamdir = streamdir->answer;
   G_debug(0,"DEBUG streamdir %s",streamdir->answer);
   G_debug(0,"DEBUG opt %s",opt->streamdir);

leads to

GRASS 6.5.svn (spearfish60):> r.terraflow elev=elevation.10m \
     filled=elevation10m.filled dir=elevation10m.mfdir \
     swatershed=elevation10m.watershed accumulation=elevation10m.accu \
     tci=elevation10m.tci d8cut=500 memory=800
stats=elevation10mstats.txt\
     STREAM_DIR=/tmp --o
...
D0/0: DEBUG streamdir /var/tmp/
D0/0: DEBUG opt /var/tmp/
...
}}}

Confusing... Interestingly, if I use (note the d typo):

{{{
r.terraflow elev= ... STREAM_dDIR=/tmp
}}}

the module does not fail. Apparently parse_args() fails?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

Interestingly, when I change
{{{
@@ -128,6 +128,6 @@
    /* temporary STREAM path */
    struct Option *streamdir;
    streamdir = G_define_option() ;
- streamdir->key = "STREAM_DIR";
+ streamdir->key = "stream_dir";
    streamdir->type = TYPE_STRING;
}}}

it works!

Question: change to lowercase which breaks the user interface (but
uppercase fails)?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

Fixed in GRASS 7, r36632 (stream_dir is already lowercase there). Remains
open for GRASS 6.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by mmetz):

Replying to [comment:9 neteler]:
> Fixed in GRASS 7, r36632 (stream_dir is already lowercase there).
Remains open for GRASS 6.

Nice!

Just a few comments:

/var/tmp does not exist on Windows, so this is not really a good choice
for default temp directory.

It is ok to call G_tempfile() in
{{{
streamdir->answer = G_store(dirname(G_tempfile()));
}}}
because this happens in r.terraflow after G_gisinit() is called, not
before.

AFAIKT, G_tempfile() only returns a filename, a file is not created (the
description of G_tempfile() says so).

Therefore I would opt to change to standard grass policy and use
dirname(G_tempfile()) as suggested first by Markus N, because that
directory exists always, independent of the OS.

Increasing the memory option to e.g. 75% doesn't really help for massive
grids (the main aim of r.terraflow) unless there are much more than 8GB of
RAM available (e.g. a cluster with 64GB RAM). Most users don't use a
cluster and probably have systems with something between 2GB and 8GB RAM;
in these cases the stream_dir is needed, with enough disk space.

Last comment: any reason why STREAMTMP_DIR is set system-wide and not with
G_putenv()? Code in question is here:

[http://trac.osgeo.org/grass/browser/grass/branches/develbranch_6/raster/r.terraflow/main.cc#L503
grass/branches/develbranch_6/raster/r.terraflow/main.cc#L503]

Markus M

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by adanner):

Replying to [comment:5 hamish]:
> the bugfix part of it looks good:
> {{{
> -flowStream = new
AMI_STREAM<waterWindowBaseType>("/var/tmp/flowStream");
> +flowStream = new AMI_STREAM<waterWindowBaseType>(streamdir->answer);
> }}}
>
>

Hmm. /var/tmp/flowStream is a file but streamdir->answer is supposed to be
a directory. "flowStream" should be appended to streamdir->answer. Can you
use strcat or is there a G_? function for that?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by glynn):

Replying to [comment:11 adanner]:

> Hmm. /var/tmp/flowStream is a file but streamdir->answer is supposed to
be a directory. "flowStream" should be appended to streamdir->answer. Can
you use strcat or is there a G_? function for that?

We normally use sprintf(), e.g.:

{{{
     char path[GPATH_MAX];
     ...
     sprintf(path, "%s/flowStream", streamdir->answer);
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by neteler):

Patch updated once more.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:9 neteler]:
> > Question: change to lowercase which breaks the user interface
> > (but uppercase fails)?

that's very weird.

> > Fixed in GRASS 7, r36632 (stream_dir is already lowercase
> > there). Remains open for GRASS 6.

since it was always broken in the past, maybe it doesn't hurt.
On the other hand, changing it will break scripts (which will now work as
expected instead of nooping).

Replying to [comment:10 mmetz]:
> /var/tmp does not exist on Windows, so this is not really a
> good choice for default temp directory.

that is a problem. If it were decided to keep it out of $MAPSET/.tmp/ we
could use a compiler macro for C:\Temp\Grass\, like we do for /dev/null
and NUL:.

ISTR the output files are of some interest after the module has ended. is
this the case? if so, putting them deep in a hidden .tmp/ dir isn't very
nice.

otherwise the lack of /var/tmp/ on MS-win makes a compelling case for
moving it to $M/.tmp/ and short-circuiting this discussion.

> It is ok to call G_tempfile() in
> {{{
> streamdir->answer = G_store(dirname(G_tempfile()));
> }}}
> because this happens in r.terraflow after G_gisinit() is called, not
before.

sorry, that should have read "before G_parser() has run", not G_gisinit().

see: http://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/parser.c#L724

{{{"The only functions which can legitimately be called before G_parser()
are: ..."}}}

> AFAIKT, G_tempfile() only returns a filename, a file is not
> created (the description of G_tempfile() says so).

don't trust the comments, test it. from past experience I think it
creates a file. (at least the `g.tempfile` module does).

> Increasing the memory option to e.g. 75% doesn't really help
> for massive grids (the main aim of r.terraflow) unless there
> are much more than 8GB of RAM available

I am not sure, but I have the memory that it was not a 1:1 trade
off, a bit more memory meant much less disk space? (untested)
what I do remember is that when I had do do a real world run with a
massive grid, I had to crank that up until I didn't hit the LFS limit of
my 32bit system, which in turn made it run faster. I might have also had
to compromise on the resolution that time, but I don't think so as I don't
like doing that & so would have remembered :slight_smile:

> Last comment: any reason why STREAMTMP_DIR is set system-wide
> and not with G_putenv()? Code in question is here:
>
>
http://trac.osgeo.org/grass/browser/grass/branches/develbranch_6/raster/r.terraflow/main.cc#L503

an environmental variable can not escape its process to the rest of the
system (can not export to parent or siblings, only self and children).
anyway, the code at lib/gis/putenv.c shows that G_putenv() is just a
wrapper incase putenv() doesn't exist on that system.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by mmetz):

Replying to [comment:14 hamish]:
> Replying to [comment:10 mmetz]:
> > /var/tmp does not exist on Windows, so this is not really a
> > good choice for default temp directory.
>
> that is a problem. If it were decided to keep it out of $MAPSET/.tmp/ we
could use a compiler macro for C:\Temp\Grass\, like we do for /dev/null
and NUL:.
>
> ISTR the output files are of some interest after the module has ended.
is this the case? if so, putting them deep in a hidden .tmp/ dir isn't
very nice.

All temporary files are deleted upon successful termination, I checked.
There is also a warning to remove any temp files in case of error:

THESE INTERMEDIATE STREAMS WILL NOT BE DELETED IN CASE OF ABNORMAL
TERMINATION OF THE PROGRAM. TO SAVE SPACE PLEASE DELETE THESE FILES
MANUALLY!

>
> otherwise the lack of /var/tmp/ on MS-win makes a compelling case for
moving it to $M/.tmp/ and short-circuiting this discussion.
>
> > It is ok to call G_tempfile() ...
>
> sorry, that should have read "before G_parser() has run", not
G_gisinit().
>
> see:
http://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/parser.c#L724

OK, then leave it blank and after G_parser() use G_tempfile() or something
else that works on all platforms if no answer was given?

>
> > AFAIKT, G_tempfile() only returns a filename, a file is not
> > created (the description of G_tempfile() says so).
>
> don't trust the comments, test it. from past experience I think it
> creates a file. (at least the `g.tempfile` module does).
>

"don't trust the comments", uff. Checking every library function if it
behaves as described in the comments is a bit uncomfortable when using
these functions in some code. In this case, g.tempfile calls the same
function like G_tempfile() and then, after that, actually creates a temp
file. So I haven't tested it, but according to the code it looks like
G_tempfile() is indeed only returning a file name without creating a file.

>
> > Increasing the memory option to e.g. 75% doesn't really help
> > for massive grids (the main aim of r.terraflow) unless there
> > are much more than 8GB of RAM available
>
> I am not sure, but I have the memory that it was not a 1:1 trade
> off, a bit more memory meant much less disk space? (untested)
> what I do remember is that when I had do do a real world run with a
massive grid, I had to crank that up until I didn't hit the LFS limit of
my 32bit system, which in turn made it run faster. I might have also had
to compromise on the resolution that time, but I don't think so as I don't
like doing that & so would have remembered :slight_smile:

The manual says that 2x80 bytes per cell are needed as disk space, no
mentioning of more memory meaning less disk space needed. It would help if
one of the authors of r.terraflow and/or the iostream library could
comment on that.

>
>
> > Last comment: any reason why STREAMTMP_DIR is set system-wide
> > and not with G_putenv()?
>
> an environmental variable can not escape its process to the rest of the
system (can not export to parent or siblings, only self and children).
anyway, the code at lib/gis/putenv.c shows that G_putenv() is just a
wrapper incase putenv() doesn't exist on that system.

That is cosmetics anyway, and I think I meant G_getenv/G_setenv, not
G_putenv. And I don't know much about environment variables, it was
essentially about why not using lib/gis functions for cross-platform
compatibility.

Markus M

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by glynn):

Replying to [comment:14 hamish]:

> > /var/tmp does not exist on Windows, so this is not really a
> > good choice for default temp directory.
>
> that is a problem. If it were decided to keep it out of $MAPSET/.tmp/ we
could use a compiler macro for C:\Temp\Grass\, like we do for /dev/null
and NUL:.

A function, e.g. G_temp_dir() would be more useful.

And we really ought to separate out the intended use of G_tempfile()
(atomic updates) from its common use (arbitrary temporary files).

> ISTR the output files are of some interest after the module has ended.
is this the case? if so, putting them deep in a hidden .tmp/ dir isn't
very nice.
>
> otherwise the lack of /var/tmp/ on MS-win makes a compelling case for
moving it to $M/.tmp/ and short-circuiting this discussion.

Note that the temporary directory also includes the hostname, in case the
database is shared via NFS. PIDs are only unique per-host, so a PID alone
doesn't guarantee uniqueness for networked filesystems.

> > It is ok to call G_tempfile() in
{{{
streamdir->answer = G_store(dirname(G_tempfile()));
}}}
> > because this happens in r.terraflow after G_gisinit() is called, not
before.
>
> sorry, that should have read "before G_parser() has run", not
G_gisinit().
>
> see:
http://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/parser.c#L724
>
> {{{"The only functions which can legitimately be called before
G_parser() are: ..."}}}
>
> > AFAIKT, G_tempfile() only returns a filename, a file is not
> > created (the description of G_tempfile() says so).

Ideally, you shouldn't assume that the "GRASS environment" ($GISRC, VAR,
WIND, etc) is valid until after G_parser() has returned. If you want
context-dependent defaults, leave ->answer as NULL then assign the value
after G_parser() has returned.

Lots of code violates this rule (e.g. modules which use the DBMI typically
fill in default values for the database options), but we should be
eliminating this rather than adding to it.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/520#comment:16&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords: r.terraflow, tmp
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Changes (by hamish):

  * keywords: => r.terraflow, tmp

Comment:

maybe part of this bug was fixed by r37295 (mid May). From July 2008
G_parser() in 6.4 and 6.5 was silently doing nasty things to option
settings in memory for options with upper case letters in them. This went
mostly unnoticed in r.terraflow (although some weirdness was noted) as
that option is seldom used and it is the only module with an upper case
option. a number of addon module users got bitten by this in their own
scripts to worse effect.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:17&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords: r.terraflow, tmp
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by mmetz):

Replying to [comment:17 hamish]:
> maybe part of this bug was fixed by r37295 (mid May). From July 2008
G_parser() in 6.4 and 6.5 was silently doing nasty things to option
settings in memory for options with upper case letters in them.

Not respecting a user-specified STREAM_DIR option was apparently a bug of
G_parser(), not r.terraflow. The default answer of /var/tmp or another
system TEMP folder is IMHO not a good idea because of lack of space. Why
not leaving it blank, if no answer is given, use the GRASS temp directory?
That directory should always exist and I guess there is most of the time
enough space.

Markus M

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:18&gt;
GRASS GIS <http://grass.osgeo.org>

#520: r.terraflow/iostream does not respect STREAM_DIR option
---------------------+------------------------------------------------------
  Reporter: mmetz | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords: r.terraflow, tmp
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:18 mmetz]:
> Not respecting a user-specified STREAM_DIR option was apparently
> a bug of G_parser(), not r.terraflow.

actually it was hardcoded, MN just fixed it: r38256, L584

the G_parser() memory bug possibly made the earlier debugging effort very
confusing because if the option was left blank the last given value was
used anyway(?)

> The default answer of /var/tmp or another system TEMP folder
> is IMHO not a good idea because of lack of space. Why not
> leaving it blank, if no answer is given, use the GRASS temp
> directory? That directory should always exist and I guess there
> is most of the time enough space.

this has been discussed before, check the archives. ISTR that the
Terraflow team (Andrew? Laura?) explained why they used that a long time
ago (I now forget exact reason), but in the more recent discussion using
/home/mapset/.tmp/ had some popular support. Note that these temp files
may be >2gig (use the memory= option to lower those needs), and they may
be potentially useful(?) for inspection/archive/debugging/reuse later, so
keeping them completely hidden may not be desired.

The current %TEMP% solution on ms-windows seems reasonable to me.
I do not feel too strongly about the choice of /tmp/, /var/tmp/, or
G_tempfile() for GRASS 7 on unix.

Just that if we use a G_tempfile() solution we would be best to have a new
G_tempdir() function instead of using possibly non-portable filesystem
tricks for that.

you can read some of my earlier bitching and moaning about that here:
http://thread.gmane.org/gmane.comp.gis.grass.devel/17750/focus=17762

regards,
Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/520#comment:19&gt;
GRASS GIS <http://grass.osgeo.org>