Skip to content

Correctness: opt-in deterministic conv, pruning empty-mask guard, TensorField backprop regression test - #17

Merged
alpsaur merged 1 commit into
masterfrom
fix/correctness-trio
Jul 17, 2026
Merged

Correctness: opt-in deterministic conv, pruning empty-mask guard, TensorField backprop regression test#17
alpsaur merged 1 commit into
masterfrom
fix/correctness-trio

Conversation

@alpsaur

@alpsaur alpsaur commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Three correctness items in the Python layer plus regression tests. No src/*.cu changes; existing built backend is unchanged.

1. Opt-in deterministic convolution (upstream #554, #504)

Convolution output depends on the input coordinate ordering: atomicAdd accumulation and kernel-map dispatch order mean that running the same conv on the same point set in a different row order yields low-order-bit differences, worst at stride > 1 and transpose. The community-confirmed workaround is to sort coordinates before the conv.

  • New MinkowskiEngine/utils/determinism.py:
    • sorted_coordinates(sparse_tensor) returns a new SparseTensor with rows in canonical lexicographic coordinate order. Implemented as a stable per-column radix argsort (batch column most significant) so it never packs coords into a single integer rank (overflow-free), and it is correct for arbitrary spatial dimension D and negative coordinates.
    • Process-wide set_deterministic(mode) / is_deterministic() flag (off by default).
  • MinkowskiConvolution / MinkowskiConvolutionTranspose forward route the input through sorted_coordinates when the flag is on (skipped for the use_mm, kernel_volume == 1, path which is a plain matmul). Documented cost: one coordinate sort per conv; intended for reproducibility/debugging, not throughput.
  • Exported as ME.set_deterministic, ME.is_deterministic, ME.sorted_coordinates.

Default behavior is unchanged (opt-in only).

2. Pruning empty-mask guard (upstream #579)

Reproduced on this fork first: an all-False mask through MinkowskiPruning on CUDA followed by a downstream conv and backward. It does not crash. The CUDA pruning kernel already emits a valid empty SparseTensor for the size-0 case, downstream conv produces zero-size outputs, and backward yields finite (empty, zero) gradients. Per the plan, no code change was needed; added a regression test that locks in the behavior.

3. TensorField backprop regression test (upstream #395)

The reported merge_sort cudaErrorIllegalAddress in inverse_mapping backward was fixed upstream by commit 02fc608, which is in this fork's history. Added a field -> sparse -> conv -> slice -> field backward test (~50k points, D=3, batch 2) run over 5 iterations with varying N to catch intermittence; asserts finite gradients. Passes (no xfail required).

Tests

New CUDA-gated modules (skip cleanly on CPU-only CI):

  • tests/python/determinism.py
  • tests/python/pruning_empty.py
  • tests/python/tensorfield_backward.py

Verified on RTX 5090 / CUDA 12.8:

  • New tests: 6 passed on GPU, 6 skipped on CPU-only.
  • Core subset convolution.py (minus TestPCD), pruning.py, half_precision.py: 26 passed, 4 skipped, 2 deselected.

… backprop test

Three correctness items for MinkowskiEngine convolution/pruning/tensor-field.

1. Opt-in deterministic convolution (upstream NVIDIA#554, NVIDIA#504)
   Conv output depends on input coordinate ordering (atomicAdd accumulation +
   kernel-map dispatch order), so repeated runs on the same unsorted input
   differ in low-order bits, worst at stride>1 / transpose. Add an opt-in
   switch that canonicalizes coordinate order before the backend call:
   - MinkowskiEngine/utils/determinism.py: sorted_coordinates(sparse_tensor)
     returns a new SparseTensor with rows in lexicographic coordinate order
     (stable per-column radix argsort, overflow-free, handles arbitrary D and
     negative coords), plus a process-wide set_deterministic/is_deterministic
     flag.
   - MinkowskiConvolution / MinkowskiConvolutionTranspose forward sort their
     input through the helper when the flag is on (skipped for the use_mm
     kernel_volume==1 path). Default behavior unchanged; documented perf cost
     of one sort per conv.
   - Exported as ME.set_deterministic / ME.is_deterministic /
     ME.sorted_coordinates.

2. Pruning empty-mask guard (upstream NVIDIA#579)
   Reproduced on this fork: an all-False mask through MinkowskiPruning on CUDA
   already returns a valid empty SparseTensor; downstream conv and backward run
   cleanly with finite (empty) grads (src/pruning_gpu.cu handles the size-0
   case). No code change needed; add a regression test that locks in the
   behavior.

3. TensorField backprop regression test (upstream NVIDIA#395)
   The merge_sort illegal-address crash in inverse_mapping backward was fixed
   upstream by 02fc608 (in our history). Add a field -> sparse -> conv -> slice
   -> field backward test over several iterations with varying N (~50k, D=3,
   batch 2) asserting finite gradients.

Tests are CUDA-gated (skipUnless) and skip cleanly on CPU CI.
@alpsaur
alpsaur merged commit 4553496 into master Jul 17, 2026
9 checks passed
@alpsaur
alpsaur deleted the fix/correctness-trio branch July 17, 2026 04:38
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