Skip to content

Preserve zero descriptors in RootSIFT normalization - #4891

Merged
ahojnnes merged 1 commit into
colmap:mainfrom
aswanth-07:fix/rootsift-zero-descriptors
Oct 27, 2026
Merged

ahojnnes merged 1 commit into
colmap:mainfrom
aswanth-07:fix/rootsift-zero-descriptors

Conversation

@aswanth-07

Copy link
Copy Markdown
Contributor

L1-Root normalization turns an all-zero descriptor row into NaNs by dividing by its zero L1 norm. The bundled VLFeat descriptor code preserves zero histograms, including those computed from flat gradients, and the sibling L2 normalizer also keeps zero rows finite.

Skip normalization for zero-norm rows. Positive rows retain the same arithmetic and byte-descriptor output. The change is in place and leaves descriptor shapes, storage formats and the default normalization choice unchanged.

Validation:

  • All 12 native feature utility tests pass. The new mixed-row regression fails on pristine main; all 11 existing cases pass there.
  • A native probe using the bundled VLFeat implementation fails before the fix and passes afterward. Twelve positive/empty matrix controls preserve exact normalized values and bytes.
  • clang-format --dry-run --Werror passes both changed files. Local Clang-Tidy 23 reports the same five unchanged-header diagnostics on base and patch; upstream uses Clang 18.

I ran a focused Windows native build, not a full application or GPU build.

AI assistance disclosure: AI-assisted development tools were used during investigation and implementation. The reported tests were run against the final diff.

@aswanth-07
aswanth-07 marked this pull request as ready for review October 24, 2026 01:00
@ahojnnes
ahojnnes enabled auto-merge (squash) October 27, 2026 09:57
@ahojnnes
ahojnnes merged commit 9371f7b into colmap:main Oct 27, 2026
16 checks passed
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 3, 2026
## Summary

Merge upstream 9371f7b and PR4895
maintainer revision db7586c into
d297c1e, retaining independent stream,
blocking-sync, LoMa resize/transfer and ONNX stream changes.

Retain upstream option initialization, CUDA diagnostics and RootSIFT zero-row
safety. Use the maintainer's generic fail-closed BF16 helper and thin LoMa
wrapper. Register CUDA tests once, keep their CUDA dependency explicit, and
fix the sparse-matrix test's mixed CMake link signature.

## Validation

Incremental over d297c1e; retained the imported merge parents.

22 affected CPU objects compiled and CLI linked incrementally;
63 focused CPU tests and 7 RTX3080Ti CUDA tests pass, zero skips/failures.
Actual separate CUDA-enabled CMake configure/generation passes. The initial
ONNX test-link and Poisson CMake failures were corrected; only outstanding
test links were completed, with no repeated CLI or object build.

The one-line Poisson test CMake fix was configured after native tests and
changes no compiled production code. Public formatting changes whitespace
and include ordering only. CUDA objects are reused for declaration/comment
header deltas. Nine inherited archive identities match qualified controls;
the remaining inputs inherit the frozen native-build link closure.

No performance, memory, whole-scene quality, HIP, Python-binding or
unsupported-device end-to-end LoMa claim. PR4895 remains unmerged upstream.

AI disclosure: Totally vibe coded.

## Upstream commits

- `5ff7e73c4efa1fcad0a7766f0242ed8cec700831` Avoid lambdas as default member initializers in feature options (MSVC ODR violation) (colmap#4900)
- `3dd12895f96ae88f43eb7cfee8b26c467c6954e1` Fix CLI option names in the feature and FAQ docs (colmap#4901)
- `15526bb9a7bca6d1843b8b849169d7f6ce4f900b` Improve CUDA driver failure diagnostics (colmap#4903)
- `9371f7b47ebe660dfa8a8d968ff8def199ef0456` Preserve zero descriptors in RootSIFT normalization (colmap#4891)
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.

2 participants