Skip to content

Stabilize 3D radical/power pruning across architectures by inflating max_radius by 1 ULP#45

Open
IvanChernyshov wants to merge 1 commit intochr1shr:devfrom
IvanChernyshov:fix-voro-dev
Open

Stabilize 3D radical/power pruning across architectures by inflating max_radius by 1 ULP#45
IvanChernyshov wants to merge 1 commit intochr1shr:devfrom
IvanChernyshov:fix-voro-dev

Conversation

@IvanChernyshov
Copy link

Fixes #43

Context

Summary
This PR fixes a platform-/codegen-dependent robustness issue in the 3D radical (power diagram) containers. In some builds (notably on macOS/ARM64, and also reproducible on Linux with more aggressive FP codegen), the computation can become under-clipped: a cell misses required cutting planes, causing non-reciprocal neighbor relationships and (in a fully periodic orthorhombic box) a total volume sum that exceeds the box volume.

Root cause (short)
In radical mode Voro++ uses the global maximum particle radius max_radius as part of conservative cutoff/pruning tests. For the particle that actually has the maximum radius, the radical cutoff term becomes exactly:

r_mul = r_i^2 - max_radius^2 = 0

This makes several pruning inequalities knife-edge. Small floating-point differences between platforms/compilers/optimization settings can flip borderline > decisions, causing required blocks/planes to be skipped. The author’s note is also relevant here: occasional rounding issues can disrupt convexity. (Negative neighbor IDs can be valid in periodic domains due to a particle’s periodic image; the failure here is additionally evidenced by broken reciprocity and the volume-sum mismatch.)

Fix
Store max_radius slightly above the observed maximum radius (1 ULP) using nextafter:

max_radius = nextafter(r, HUGE_VAL);

This makes r_mul slightly negative for the max-radius particle, turning the knife-edge comparisons into strictly conservative ones. It can only make pruning slightly less aggressive (safe), and has negligible performance impact in practice.

Scope
This PR is intentionally limited to the 3D radical/power containers and updates all 3D code paths that assign/update max_radius, including ghost-cell helpers. (No 2D changes are included here.)

Verification
The reproduction repo contains:

  • the minimal repro program and dataset,
  • CI steps that run original vs patched on Ubuntu and macOS,
  • and an additional Linux “aggressive flags” build that reproduces the same failure mode and shows it fixed with this change.

@IvanChernyshov
Copy link
Author

Quick note: while validating the same root-cause pattern, I also found that the 2D radical/power containers on dev can exhibit the same platform/codegen sensitivity (same "knife-edge max_radius / pruning" behavior under aggressive FP codegen). I prepared a minimal 2D repro + CI + patch (same max_radius = nextafter(...) idea) here: https://github.com/IvanChernyshov/voro_repro_2d

I'm not opening a separate issue/PR for 2D yet. How would you prefer to handle it? I can either extend this PR to include the analogous 2D changes as well, or keep this PR strictly 3D-focused and open a separate follow-up PR for 2D after the 3D fix is reviewed/merged.

Please let me know which option you prefer.

P.S. Small question: does 2D patch need not be applied to src/ only, or should also include the legace 2d/ subtree?

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.

1 participant

Comments