Skip to content

Fix randomly failing sample consensus test - #6475

Merged
larshg merged 1 commit into
PointCloudLibrary:masterfrom
mvieth:fix_test_sample_consensus
Sep 4, 2026
Merged

Fix randomly failing sample consensus test#6475
larshg merged 1 commit into
PointCloudLibrary:masterfrom
mvieth:fix_test_sample_consensus

Conversation

@mvieth

@mvieth mvieth commented Sep 3, 2026

Copy link
Copy Markdown
Member

The problem seems to be a lost wakeup: LMedS only does very few iterations (much fewer than the other sac methods) and sac.computeModel() accordingly returns quickly. In rare cases, it seems to be done before the main thread is listening/waiting for the condition_variable notification, so it is missed and the test fails after the timeout. The fix is to add a bool done that is set to true after sac.computeModel() is finished, and use the other overload of cv.wait_for() that returns the value of done (only true if finished before the timeout).

Reverts 11ab21b because the verbose output is no longer necessary.

See e.g. https://www.modernescpp.com/index.php/c-core-guidelines-be-aware-of-the-traps-of-condition-variables/

The problem seems to be a lost wakeup: LMedS only does very few iterations (much fewer than the other sac methods) and sac.computeModel() accordingly returns quickly. In rare cases, it seems to be done before the main thread is listening/waiting for the condition_variable notification, so it is missed and the test fails after the timeout. The fix is to add a `bool done` that is set to true after sac.computeModel() is finished, and use the other overload of cv.wait_for() that returns the value of `done` (only true if finished before the timeout).
@mvieth mvieth added module: test module: sample_consensus changelog: fix Meta-information for changelog generation labels Sep 3, 2026

@larshg larshg 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.

Looks good 👍

@larshg
larshg merged commit a3fa3f1 into PointCloudLibrary:master Sep 4, 2026
11 of 13 checks passed
@mvieth
mvieth deleted the fix_test_sample_consensus branch September 4, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: fix Meta-information for changelog generation module: sample_consensus module: test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants