Add error handling for insufficient correspondences in AdvancedMatching#7234
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a precondition check in AdvancedMatching to ensure at least three correspondences before running the tuple test, avoiding runtime errors and ineffective processing. Also updates the CHANGELOG to document this new behavior.
- Add guard clause in
AdvancedMatchingforncorr <= 2 - Update CHANGELOG entry for this bug fix
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cpp/open3d/pipelines/registration/FastGlobalRegistration.cpp | Added error handling and early return for insufficient correspondences in AdvancedMatching. |
| CHANGELOG.md | Documented the new error handling for AdvancedMatching. |
Comments suppressed due to low confidence (1)
cpp/open3d/pipelines/registration/FastGlobalRegistration.cpp:72
- There are no unit tests covering this new guard for insufficient correspondences. Please add tests to verify behavior when
ncorr < 3.
if (ncorr <= 2) {
| int number_of_trial = ncorr * 100; | ||
|
|
||
| if (ncorr <= 2) { | ||
| utility::LogError( |
There was a problem hiding this comment.
utility::LogError throws an exception by default, so the subsequent return {} is unreachable. Consider using a non-throwing logger (e.g., LogWarning) or removing the unreachable return.
| - Add optional indices arg for fast computation of a small subset of FPFH features (PR #7118). | ||
|
|
||
| - Add error handling for insufficient correspondences in AdvancedMatching (PR #7234) | ||
|
|
There was a problem hiding this comment.
There's an empty list item (- ) before the new changelog entry; remove it to maintain consistent formatting.
| int number_of_trial = ncorr * 100; | ||
|
|
||
| if (ncorr <= 2) { | ||
| utility::LogError( |
There was a problem hiding this comment.
Hi @sutatoruta thanks for catching this! Can you update the Error to a Warning? LogError will throw an exception and disrupt calling code. Use it only when there is no valid result that we can return. Here {} is a valid result.
Type
Motivation and Context
The tuple test within AdvancedMatching requires at least 3 correspondences, but this requirement wasn't previously checked. This lack of validation led to an error during random number generation specifically when there were no correspondences. While having 1 or 2 correspondences did not cause this specific error, processing them is also ineffective for this test.
This PR implements the check for the minimum requirement of 3 correspondences. This prevents the error observed with 0 correspondences and also avoids unproductive processing for the ineffective cases (1 or 2 correspondences), without otherwise altering the behavior.
Checklist:
python util/check_style.py --applyto apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description