Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GDALRasterComputeMinMaxLocation / GDALRasterBand::ComputeMinMaxLocation, and map it to SWIG #11200

Merged
merged 8 commits into from
Nov 10, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Nov 3, 2024

and add a gdal_minmax_location.py script:

$ python ../swig/python/gdal-utils/osgeo_utils/samples/gdal_minmax_location.py byte.tif
Minimum=74.0 at (col,line)=(9,17), (X,Y)_georef=(441290.0,3750270.0), (long,lat)_WGS84=(-117.6358076,33.8929309)
Maximum=255.0 at (col,line)=(2,18), (X,Y)_georef=(440870.0,3750210.0), (long,lat)_WGS84=(-117.6403456,33.8923662)

(on top of PR #11199)

… to find the min/max elements in a buffer

Refs qgis/QGIS#59285

NaN values are taken into account for float/double
Contains a SSE2 optimized version for int8/uint8/int16/uint16/int32/uint32/float/double in the no-nodata case (also taking into account NaN)
@rouault rouault added this to the 3.11.0 milestone Nov 3, 2024
@rouault rouault force-pushed the GDALRasterComputeMinMaxLocation branch 5 times, most recently from dc1a0ee to 505660f Compare November 3, 2024 13:25
@coveralls
Copy link
Collaborator

coveralls commented Nov 3, 2024

Coverage Status

coverage: 69.473% (+0.05%) from 69.428%
when pulling 58d4d3e on rouault:GDALRasterComputeMinMaxLocation
into f5eedd2 on OSGeo:master.

#ifdef NOT_EFFICIENT

/************************************************************************/
/* minmax_element() */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there are algorithms for finding both the min and max element of a list of numbers at the same time that beat the speed of iterating the list and checking each number versus min and each numeber versus mac and updating the min and max as you go. You can do less comparisons by pulling out two numbers at a time, comparing them to each other, and checking the min one to overall min and the max one versus overall max. I think this ends up doing 3/4's the number of overall comparisons than the naive approach. I have lost the reference for this but I coded it in my java library. See here; https://github.com/bdezonia/zorbage/blob/master/src/main/java/nom/bdezonia/zorbage/algorithm/MinMaxElement.java

Copy link
Member Author

@rouault rouault Nov 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion but saving 1/4 of operations isn't going to improve performance in a significant way.

Current timings are for a Byte vector of 10 millions elements (lower is better):

  • using std::minmax_element(): 15,062,444
  • using NOT_EFFICIENT code path: 9,625,260
  • using SSE2 optimized gdal::min_element() + gdal::max_element() (enabled currently): 1,341,901

So your suggestion would perhaps reduce NOT_EFFICIENT code path timing, but it would be still far away from the twice repeated SSE2 optimized code paths. Actually I've just tested it and it results in... 50% slower code than the naive version... Presumably due to degraded pipelining

…6, int16, uint32, int32, float and double nodata cases
@rouault rouault force-pushed the GDALRasterComputeMinMaxLocation branch from 505660f to 9a9ffc3 Compare November 3, 2024 20:52
…the non-SSE2 case, as it turns out at least on Apple Silicon that our 'optimized' version is generally slower
…cation, and map it to SWIG

and add a gdal_minmax_location.py script:

```
$ python ../swig/python/gdal-utils/osgeo_utils/samples/gdal_minmax_location.py byte.tif
Minimum=74.0 at (col,line)=(9,17), (X,Y)_georef=(441290.0,3750270.0), (long,lat)_WGS84=(-117.6358076,33.8929309)
Maximum=255.0 at (col,line)=(2,18), (X,Y)_georef=(440870.0,3750210.0), (long,lat)_WGS84=(-117.6403456,33.8923662)
```
@rouault rouault force-pushed the GDALRasterComputeMinMaxLocation branch from 9a9ffc3 to 58d4d3e Compare November 3, 2024 23:45
@rouault rouault merged commit d107444 into OSGeo:master Nov 10, 2024
36 checks passed
@jratike80
Copy link
Collaborator

The gdal-dev version from OSGeo4W installed today shows version GDAL 3.11.0dev-d9b65d7609, released 2024/11/05. The date is too old for this minmax feature, but if the version string refers to the number of last included commit, the d9b65d7 removed the Lura driver and it was a few days later. So maybe the GDALRasterComputeMinMaxLocation should be available. However, running gdal_minmax_location.py prints an error:
AttributeError: 'Band' object has no attribute 'ComputeMinMaxLocation'

Should I wait for new nightly builds, or could there be something wrong in the OSGeo4W?

@rouault
Copy link
Member Author

rouault commented Nov 14, 2024

GDAL 3.11.0dev-d9b65d7609, released 2024/11/05

@jef-n Is the date of the system where OSGeo4W is build up-to-date ?

so maybe the GDALRasterComputeMinMaxLocation should be available

yes, it should be available. Commit d9b65d7 is the merge request from yesterday, so it must include GDALRasterComputeMinMaxLocation. Are you sure your PYTHONPATH is correctly set to point to the updated GDAL python bindings?

@jratike80
Copy link
Collaborator

jratike80 commented Nov 14, 2024

My fault. I have learned to activate gdal-dev in OSGeo4W by running command gdal-dev-env. For activating both gdal-dev and gdal-dev python bindings I must run gdal-dev-py-env.

gdal_minmax_location.py runs without issues. Very fast.

Everything is obvious when one reads the contents of the gdal-dev-py-env.bat file, but I explain it shortly so I can give a link for the next user who will have the same problem.

If the OSGeo4W root directory is at C:\OSGeo4W, then GDAL python bindings are installed into
C:\OSGeo4W\apps\Python312\Lib\site-packages
GDAL-dev python bindings are installed into
C:\OSGeo4W\apps\gdal-dev\lib\site-packages
When OSGeo4W starts, it sets only PYTHONHOME=C:\OSGeo4W\apps\Python312
Running gdal-dev-py-env.bat sets PYTHONPATH=C:\OSGeo4W\apps\gdal-dev\lib\site-packages;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants