google-deepmind / google-deepmind/mujoco

filterFlexContacts can keep the wrong final contact after FPS compaction

Open
#3,297 0 comments 0 reactions 1 assignee Claimed by @quagla View on GitHub
Dominant language
C++
Stars
15.2k
Forks
1.8k
Avg merge
10d 16h
Merged PRs (30d)
25

Description

I think there may be a bug in `filterFlexContacts` introduced in `8ba68cee` (`Limit the number of flex contacts per collision pair`).

The short version is that the FPS selection loop can end up keeping a different final contact than the one it actually selected.

The relevant code is here:
- `src/engine/engine_collision_driver.c:401`
- current `main` at `5d782a2b`

What I think is happening:

- `filterFlexContacts` selects contacts with farthest-point sampling.
- During the loop it swaps `contacts[best]` into the kept prefix at `contacts[nselected]`.
- But that swap only happens when `nselected < mjMAXCONPAIR - 1`.
- On the final selected contact, the function increments `nselected` and then truncates `d->ncon`, without compacting `contacts[best]` into the kept prefix first.

So the last selected contact can be dropped, and the truncated prefix can contain a different contact instead.

A small toy translation of the loop reproduces that behavior: the selected sequence is `{A, B, C}` but the kept prefix after compaction/truncation becomes `{A, B, D}`.

I think there is also a second invariant problem here: because `contacts[]` is swapped in place during FPS, `selected[]` / `min_dist[]` no longer stay attached to the same logical candidate after the first swap. But even without leaning on that, the missing final compaction step already looks sufficient to make the retained set wrong.

Expected behavior:
- the first `nselected` contacts after filtering should be exactly the contacts selected by FPS.

Actual behavior:
- the final kept prefix can differ from the selected set.

If this diagnosis looks right, I'm happy to send a small PR with a regression test.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.