Skip to content

r.univar: Fix max of all-null maps and zones - #8099

Merged
wenzeslaus merged 2 commits into
OSGeo:mainfrom
saket0187:r-univar-null-max
Oct 22, 2026
Merged

wenzeslaus merged 2 commits into
OSGeo:mainfrom
saket0187:r-univar-null-max

Conversation

@saket0187

Copy link
Copy Markdown
Contributor

Fix r.univar reporting -DBL_MAX instead of nan (JSON null) as the maximum of all-null maps and zones, wrong since 8.4.0 because #3512 changed the start value but not the merge check; adds a pytest test that fails without the fix.

Claude was used as a coding assistant.

@github-actions github-actions Bot added raster Related to raster data processing Python Related code is in Python C Related code is in C module tests Related to Test Suite labels Oct 21, 2026
@echoix

echoix commented Oct 21, 2026

Copy link
Copy Markdown
Member

There’s a kind of outage when installing Ubuntu packages it seems. Let’s wait before trying to trigger new runs (I reran a couple and still failed)

@wenzeslaus wenzeslaus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could add the min test as suggested in my comment or given that both min and max can be tested and zd->bucket.n is thread-local, the lock can be acquired only later (hopefully slightly faster than the current code on main). The update test then continues to use the values in question.

// (Untested code)
if (zd->bucket.n != 0) {
#if defined(_OPENMP)
    omp_set_lock(&minmax[z]);
#endif
    // The max and min values are tested for NaN (their defaults).
    if (stats[z].max < zd->max || stats[z].max != stats[z].max)
        stats[z].max = zd->max;
    if (stats[z].min > zd->min || stats[z].min != stats[z].min)
        stats[z].min = zd->min;
#if defined(_OPENMP)
    omp_unset_lock(&minmax[z]);
#endif
}

Comment thread raster/r.univar/tests/r_univar_test.py Outdated
Comment thread raster/r.univar/r.univar_main.c Outdated
@saket0187
saket0187 requested a review from wenzeslaus October 21, 2026 17:22

@wenzeslaus wenzeslaus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! It is very important to keep improving r.univar, not only important to users, but we use that a lot in tests as well.

@wenzeslaus
wenzeslaus enabled auto-merge (squash) October 22, 2026 02:43
@wenzeslaus
wenzeslaus merged commit 34d5788 into OSGeo:main Oct 22, 2026
28 of 42 checks passed
@github-actions github-actions Bot added this to the 8.6.0 milestone Oct 22, 2026
@saket0187
saket0187 deleted the r-univar-null-max branch October 22, 2026 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants