[GRASS-dev] [GRASS GIS] #2867: Add modifier parameter to r.series

#2867: Add modifier parameter to r.series
-------------------------+-------------------------
Reporter: pvanbosgeo | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone:
Component: Default | Version: unspecified
Keywords: | CPU: Unspecified
Platform: Unspecified |
-------------------------+-------------------------
An useful enhancement of r.series would be to add a "modifier" (or similar
name) parameter to r.series
which would allow to calculate the statistic defined by "method" on a
modified version of the maps (e.g. squared, square root, log, sin, cos,
etc.).

Attached patch is by Moritz Lennert, see also the email thread
[https://lists.osgeo.org/pipermail/grass-dev/2016-January/078470.html\].

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone:
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by pvanbosgeo):

* Attachment "r_series_modifier.diff" added.

patch to add modifier to r.series

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by martinl):

* keywords: => r.series
* component: Default => Raster
* milestone: => 7.0.4

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [ticket:2867 pvanbosgeo]:
> An useful enhancement of r.series would be to add a "modifier" (or
similar name) parameter to r.series
> which would allow to calculate the statistic defined by "method" on a
modified version of the maps (e.g. squared, square root, log, sin, cos,
etc.).

My main concern is that this is going to open the door to a never-ending
stream of requests to add more modifiers or more features (e.g. parametric
modifiers, or selecting modifiers on a per-output bases) until we end up
having to embed either r.mapcalc or Python into r.series.

OTOH, I'm aware that the usual solution (use in conjunction with
r.mapcalc) has a significant overhead when you're potentially dealing with
hundreds of input maps. Even so, I'd be inclined to limit the available
modifiers to those which seem likely to be particularly useful (only
sqrt/x^2 and log/exp spring to mind); there are a *lot* of real->real
functions in the standard math library.

As for the patch itself, I'd suggest:

1. Determining the function during initialisation and setting a function
pointer, rather than performing the test in the inner loop. Multiple
strcmp()s inside a triple-nested loop (rows*columns*inputs) are likely to
add significant overhead.

2. Vectorising the operations so that the dispatch is only done once per
row, not per value. Even an indirect call via a function pointer may be
more expensive than the function itself (some math functions may compile
to a single FPU instruction). The simplest way to do this would be to
modify inputs.buf in-place after reading.

In that regard, maybe we should try to extract the relevant portions of
r.mapcalc to a separate library so that they can be re-used here?

Or maybe a better option would be to just package up the raster-I/O loops
as a Python module so that the actual processing can be implemented as
Python functions which convert a 2D (inputs*columns) array into a set of
1D arrays (one per output).

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by mlennert):

Replying to [comment:2 glynn]:
> Replying to [ticket:2867 pvanbosgeo]:
> > An useful enhancement of r.series would be to add a "modifier" (or
similar name) parameter to r.series
> > which would allow to calculate the statistic defined by "method" on a
modified version of the maps (e.g. squared, square root, log, sin, cos,
etc.).
>
> My main concern is that this is going to open the door to a never-ending
stream of requests to add more modifiers or more features (e.g. parametric
modifiers, or selecting modifiers on a per-output bases) until we end up
having to embed either r.mapcalc or Python into r.series.
>
> OTOH, I'm aware that the usual solution (use in conjunction with
r.mapcalc) has a significant overhead when you're potentially dealing with
hundreds of input maps. Even so, I'd be inclined to limit the available
modifiers to those which seem likely to be particularly useful (only
sqrt/x^2 and log/exp spring to mind);

+1

>
> As for the patch itself, I'd suggest:
>
> 1. Determining the function during initialisation and setting a function
pointer, rather than performing the test in the inner loop. Multiple
strcmp()s inside a triple-nested loop (rows*columns*inputs) are likely to
add significant overhead.

+1

That's why I said in the mail accompanying the patch:

"Try the attached patch, which is just a brute-force, proof-of-concept.
It would be nicer to solve this with function pointers, but I'm not at
ease with that, so I'll leave that to others."

:wink:

>
> 2. Vectorising the operations so that the dispatch is only done once per
row, not per value. Even an indirect call via a function pointer may be
more expensive than the function itself (some math functions may compile
to a single FPU instruction). The simplest way to do this would be to
modify inputs.buf in-place after reading.
>
> In that regard, maybe we should try to extract the relevant portions of
r.mapcalc to a separate library so that they can be re-used here?

If that is not too much work, then this would probably be a good thing.
But if it's only for adding a few modifiers to r.series is it worth the
effort ?

>
> Or maybe a better option would be to just package up the raster-I/O
loops as a Python module so that the actual processing can be implemented
as Python functions which convert a 2D (inputs*columns) array into a set
of 1D arrays (one per output).

You've lost me there.

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by pvanbosgeo):

Replying to [comment:2 glynn]:
> Replying to [ticket:2867 pvanbosgeo]:
> > An useful enhancement of r.series would be to add a "modifier" (or
similar name) parameter to r.series
> > which would allow to calculate the statistic defined by "method" on a
modified version of the maps (e.g. squared, square root, log, sin, cos,
etc.).
>
> My main concern is that this is going to open the door to a never-ending
stream of requests to add more modifiers or more features (e.g. parametric
modifiers, or selecting modifiers on a per-output bases) until we end up
having to embed either r.mapcalc or Python into r.series.
>
> OTOH, I'm aware that the usual solution (use in conjunction with
r.mapcalc) has a significant overhead when you're potentially dealing with
hundreds of input maps. Even so, I'd be inclined to limit the available
modifiers to those which seem likely to be particularly useful (only
sqrt/x^2 and log/exp spring to mind); there are a *lot* of real->real
functions in the standard math library.

Completely get your point.. but if I may still add one possibly very
particularly useful one: pow(). It could possible be included instead sqrt
and x^2.

>
> As for the patch itself, I'd suggest:
>
> 1. Determining the function during initialisation and setting a function
pointer, rather than performing the test in the inner loop. Multiple
strcmp()s inside a triple-nested loop (rows*columns*inputs) are likely to
add significant overhead.
>
> 2. Vectorising the operations so that the dispatch is only done once per
row, not per value. Even an indirect call via a function pointer may be
more expensive than the function itself (some math functions may compile
to a single FPU instruction). The simplest way to do this would be to
modify inputs.buf in-place after reading.
>
> In that regard, maybe we should try to extract the relevant portions of
r.mapcalc to a separate library so that they can be re-used here?
>
> Or maybe a better option would be to just package up the raster-I/O
loops as a Python module so that the actual processing can be implemented
as Python functions which convert a 2D (inputs*columns) array into a set
of 1D arrays (one per output).

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by mlennert):

Replying to [comment:4 pvanbosgeo]:
> Replying to [comment:2 glynn]:
> > Replying to [ticket:2867 pvanbosgeo]:
> > > An useful enhancement of r.series would be to add a "modifier" (or
similar name) parameter to r.series
> > > which would allow to calculate the statistic defined by "method" on
a modified version of the maps (e.g. squared, square root, log, sin, cos,
etc.).
> >
> > My main concern is that this is going to open the door to a never-
ending stream of requests to add more modifiers or more features (e.g.
parametric modifiers, or selecting modifiers on a per-output bases) until
we end up having to embed either r.mapcalc or Python into r.series.
> >
> > OTOH, I'm aware that the usual solution (use in conjunction with
r.mapcalc) has a significant overhead when you're potentially dealing with
hundreds of input maps. Even so, I'd be inclined to limit the available
modifiers to those which seem likely to be particularly useful (only
sqrt/x^2 and log/exp spring to mind); there are a *lot* of real->real
functions in the standard math library.
>
> Completely get your point.. but if I may still add a possibly very
particularly useful one: pow(). It could possible be included instead of
sqrt and x^2.

The problem with pow() is that it needs to parameters and this would
complicate things further.

Another option, maybe more in line with Glynn's suggestion of extracting
some or r.mapcalc into a library would be to create a
modification_expression parameter which would take any arbitrary formula
using r.mapcalc syntax (e.g. pow(x, 4.5) or cos(x)/sqrt(x), etc) where x
represents the map input to r.series. But IIUC that would +/- mean
recreating r.mapcalc within r.series...

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

#2867: Add modifier parameter to r.series
--------------------------+-------------------------
  Reporter: pvanbosgeo | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Raster | Version: unspecified
Resolution: | Keywords: r.series
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [comment:2 glynn]:

> In that regard, maybe we should try to extract the relevant portions of
r.mapcalc to a separate library so that they can be re-used here?

r67658 moves most of the r.mapcalc functions into lib/calc.

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