Opened 4 years ago

Last modified 7 months ago

#2867 new enhancement

Add modifier parameter to r.series

Reported by: pvanbosgeo Owned by: grass-dev@…
Priority: normal Milestone: 7.6.2
Component: Raster Version: unspecified
Keywords: r.series Cc:
CPU: Unspecified Platform: Unspecified

Description

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.

Attachments (1)

r_series_modifier.diff (1.7 KB) - added by pvanbosgeo 4 years ago.
patch to add modifier to r.series

Download all attachments as: .zip

Change History (15)

Changed 4 years ago by pvanbosgeo

Attachment: r_series_modifier.diff added

patch to add modifier to r.series

comment:1 Changed 4 years ago by martinl

Component: DefaultRaster
Keywords: r.series added
Milestone: 7.0.4

comment:2 in reply to:  description ; Changed 4 years ago by glynn

Replying to 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/x2 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.
  1. 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).

comment:3 in reply to:  2 Changed 4 years ago by mlennert

Replying to glynn:

Replying to 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/x2 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."

;-)

  1. 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.

comment:4 in reply to:  2 ; Changed 4 years ago by pvanbosgeo

Replying to glynn:

Replying to 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/x2 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 x2.

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. 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).

Last edited 4 years ago by pvanbosgeo (previous) (diff)

comment:5 in reply to:  4 Changed 4 years ago by mlennert

Replying to pvanbosgeo:

Replying to glynn:

Replying to 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/x2 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 x2.

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...

comment:6 in reply to:  2 Changed 4 years ago by glynn

Replying to 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.

comment:7 Changed 3 years ago by martinl

Milestone: 7.0.47.0.5

comment:8 Changed 3 years ago by martinl

Milestone: 7.0.57.3.0

comment:9 Changed 3 years ago by martinl

Milestone: 7.3.07.4.0

Milestone renamed

comment:10 Changed 21 months ago by neteler

Milestone: 7.4.07.4.1

Ticket retargeted after milestone closed

comment:11 Changed 16 months ago by neteler

Milestone: 7.4.17.4.2

comment:12 Changed 13 months ago by martinl

Milestone: 7.4.27.6.0

All enhancement tickets should be assigned to 7.6 milestone.

comment:13 Changed 9 months ago by martinl

Milestone: 7.6.07.6.1

Ticket retargeted after milestone closed

comment:14 Changed 7 months ago by martinl

Milestone: 7.6.17.6.2

Ticket retargeted after milestone closed

Note: See TracTickets for help on using tickets.