Skip to content

Improve CUDA driver failure diagnostics - #4903

Merged
ahojnnes merged 2 commits into
colmap:mainfrom
cola1917:fix/cuda-driver-diagnostics-4890
Oct 27, 2026
Merged

ahojnnes merged 2 commits into
colmap:mainfrom
cola1917:fix/cuda-driver-diagnostics-4890

Conversation

@cola1917

Copy link
Copy Markdown
Contributor

Summary

Addresses #4890 (CUDA diagnostics; does not claim to fix the separate Windows unhandled-exception exit behavior).

GetNumCudaDevices() currently treats cudaErrorInsufficientDriver like an ordinary no-device result, losing the reason for the subsequent device-count check failure. Preserve its zero-device return value, so callers that support CPU fallback continue to work, but log the actual failure and actionable driver/build guidance first.

Use a common error formatter in the CUDA checking helpers. It preserves the numeric error code, handles null/empty runtime descriptions, and adds driver guidance specifically for insufficient-driver errors. HIP builds receive AMD/HIP-specific guidance. Ordinary no-device/successful zero-device enumeration remains quiet.

Validation

  • Base: 5ff7e73c4efa1fcad0a7766f0242ed8cec700831.
  • Three new GTest cases: success, insufficient-driver diagnostics, and unknown error-code preservation. Against unchanged production code, two fail; with this patch, all three pass.
  • Configured the real project with CUDA enabled (CUDA 12.8, GCC 11, Release), built the new CMake test target and existing logging/string test targets, and passed all three CTest executables.
  • A real CUDA runtime in a container without an exposed driver returns error 35. The patched device enumeration emits the actionable warning and still returns zero.
  • Separate validation-only runtime interposition checks: null and empty error descriptions (all three tests pass in both modes); insufficient-driver warning; no warning for successful zero-device or no-device results. The shim is not included in the product code.
  • clang-format 23.1.3 and git diff --check pass.

Scope / limitations

  • No host-driver downgrade or modification.
  • Windows and HIP execution have not been verified locally. No full repository test run.
  • This change does not alter the CLI exception policy or pretend an insufficient driver can be recovered by GPU execution. The separate Windows silent-termination symptom remains to be investigated; the enumeration warning is emitted before the subsequent exception.
  • A GPU-exposed container probe encountered CUDA initialization error 3, so this is not presented as a successful GPU-kernel smoke test. The diagnostic tests themselves do not require a working GPU.

Implementation and validation were AI-assisted.

Comment thread src/colmap/util/cudacc_test.cc Outdated
EXPECT_NE(message.find("error " + std::to_string(static_cast<int>(
cudaErrorInsufficientDriver))),
std::string::npos);
EXPECT_NE(message.find("driver"), std::string::npos);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: gmock matchers preferred here EXPECT_THAT(message, testing::HasSubstr(...));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 3a70d3d: switched the string assertions to EXPECT_THAT with testing::HasSubstr (and testing::Not for exclusions). Test-only change; production code is unchanged. Rebuilt the CUDA-enabled test target and reran util/cudacc_test, util/logging_test, and util/string_test: all passed. Thanks for the suggestion!

@ahojnnes ahojnnes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, LGTM apart from a small suggestion.

@ahojnnes
ahojnnes enabled auto-merge (squash) October 27, 2026 09:20
@ahojnnes
ahojnnes merged commit 15526bb into colmap:main Oct 27, 2026
16 checks passed
ahojnnes added a commit that referenced this pull request Oct 28, 2026
)

Pull requests now automatically run only the fast CI jobs. The slow
Windows and pycolmap jobs run once a maintainer adds the new `full-ci`
label. On #4903, the gated jobs took about 3.4 runner-hours, and the
slowest one (the pycolmap Windows wheel, 83 min) set the time to green.

Runs automatically on every PR update:
- COLMAP (Ubuntu), COLMAP (Mac), COLMAP (Docker)
- PyCOLMAP Linux wheel (`ubuntu-24.04`) and the docs build, which
depends on it

Runs only with the `full-ci` label:
- COLMAP (Windows): `windows-2025-vs2026 Release`, `windows-2025-vs2026
Release CUDA`
- PyCOLMAP wheels: `macos-14 arm64`, `windows-2025-vs2026`,
`ubuntu-24.04 CUDA`

Details:
- The gated jobs' required status checks in the main-branch ruleset stay
"Expected" until the label is added and the jobs pass, so they still
gate merging. No ruleset changes are needed.
- The label stays on the PR, so later pushes re-run the gated jobs
automatically. Only users with triage access or higher can add labels.
- Pushes to main and release/*, releases, and manual dispatches run
everything as before.
- To gate only some pycolmap wheels, `build-pycolmap.yml` splits the
build job into `build` (Linux) and `build-gated` (macOS, Windows, CUDA).
The second job reuses the first job's name, env, and steps through YAML
anchors, so check names, artifact names, and cache keys are unchanged.
`pypi-publish` now needs both jobs.
- Adding any other label does not cancel an in-progress run, because
such label events get their own concurrency group.
- The `assigned` trigger is removed from all workflows, as it re-ran CI
whenever someone was assigned.
- `doc/contribution.rst` documents the label.

Verification performed:
- Compared the old and new workflows with YAML anchors resolved. Apart
from the intended trigger, concurrency, and `if` changes, the jobs are
identical, and the two pycolmap matrices together equal the original
matrix.
- Ran actionlint 1.7.12. It accepts the new expressions and anchors. Its
only warnings are false positives about `arch`/`deploymentTarget`
missing from the Linux-only matrix; those values were already unset for
that entry.
- This PR uses the new workflows itself: without the label, the five
gated checks should stay "Expected". Adding `full-ci` should run them.
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