Repository navigation
Improve CUDA driver failure diagnostics - #4903
Merged
Merged
Conversation
ahojnnes
reviewed
Oct 27, 2026
| EXPECT_NE(message.find("error " + std::to_string(static_cast<int>( | ||
| cudaErrorInsufficientDriver))), | ||
| std::string::npos); | ||
| EXPECT_NE(message.find("driver"), std::string::npos); |
Contributor
There was a problem hiding this comment.
nit: gmock matchers preferred here EXPECT_THAT(message, testing::HasSubstr(...));
Contributor
Author
There was a problem hiding this comment.
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
approved these changes
Oct 27, 2026
ahojnnes
left a comment
Contributor
There was a problem hiding this comment.
Thanks, LGTM apart from a small suggestion.
ahojnnes
enabled auto-merge (squash)
October 27, 2026 09:20
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses #4890 (CUDA diagnostics; does not claim to fix the separate Windows unhandled-exception exit behavior).
GetNumCudaDevices()currently treatscudaErrorInsufficientDriverlike 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
5ff7e73c4efa1fcad0a7766f0242ed8cec700831.git diff --checkpass.Scope / limitations
Implementation and validation were AI-assisted.