OSGeo
r.prominence: update from GRASS GIS 6 version
This PR
- updates https://github.com/OSGeo/grass-addons/tree/grass6/src/raster/r.prominence to GRASS GIS 7/8
- indents the C code with https://github.com/OSGeo/grass/blob/master/utils/grass_indent.sh
- updates example to North Carolina sample data set
I have done shallow testing, using the new example.
Note: r.prominence appears to run very slow, perhaps there is potential to optimize that?
GTLIB is an artefact in the Makefile. It is a pathetic attempt at simplifying some of the GRASS APIs that I made many years ago. You can safely remove the reference from the Makefile.
I looked through my local archive and noticed that I have a 1.1 revision of the r.prominence source code (the one in grass-addons is 1.0). The basic difference is that I removed the "local" and "global" normalization flags. Apparently, those were not very useful and just made everything more complicated. How do you want me to proceed?
I looked through my local archive and noticed that I have a 1.1 revision of the r.prominence source code (the one in grass-addons is 1.0). The basic difference is that I removed the "local" and "global" normalization flags. Apparently, those were not very useful and just made everything more complicated. How do you want me to proceed?
Yes, please! I just tried to get the current C code into a compilable stage, not knowing too much about the details... (I simply used this lookup table: https://trac.osgeo.org/grass/wiki/Grass7/RasterLib/ListOfFunctions)
@benducke The module could be made faster with a row cache, similar to r.neighbors. Currently each raster row is read with Rast_get_d_row() quite often (2 * radius + 2 times), which costs time.
I could help with the implementation if there is interest.
...The module could be made faster with a row cache, similar to
r.neighbors....
@metzm Since you are bringing up that module, I just wanted to make sure you are aware that the new version in OSGeo/grass#1724 adds a memory parameter (with G_OPT_MEMORYMB).
As for merging this, the "Review required" here is meant to warn merging to a wrong branch. The master is kept for now as legacy, but grass7 is currently the one which should be used.
So... I have checked my old notes on r.prominence. Turns out that, back in the day, I wrote another module "r.normalize" to perform general normalization of raster map cell values (to the ranges [0;1] or [-1;1], depending on input and preference). Back then, I considered this to be the better approach, since normalization is a generic technique, useful to compare results. So it makes sense to outsource it to a separate, more flexible module (and, in this case, to drop the normalization options from r.prominence). Let me know if you agree, and if there is no other, existing GRASS module that can serve the purpose of globally or locally (within a user-defined window) normalizing raster values, I will add "r.normalize" to the repo. It is really a very simple module and, if found useful, should require little work for including it into the main distribution. Otherwise, I will just review (and approve) r.prominence as-is.
...I will add "r.normalize" to the repo. It is really a very simple module and, if found useful, should require little work for including it into the main distribution.
A separate modules sounds great. A simple module even better. No opinion on addons versus the main repo.
This was merged to 'master', which nowadays is not in active use. #858 is a manual port to grass8.
