[BUG FIX] Warn about CoacdOptions.pca=True misalignment (closes #2747)#2757
[BUG FIX] Warn about CoacdOptions.pca=True misalignment (closes #2747)#2757voidborne-d wants to merge 1 commit intoGenesis-Embodied-AI:mainfrom
Conversation
…died-AI#2747) Upstream CoACD applies a PCA transformation to the input mesh during convex decomposition but does not transform the resulting hulls back, so collision bodies end up misaligned with the original mesh and otherwise stable scenes collapse on contact. Add a model_validator on CoacdOptions that emits a clear warning referencing the upstream issue (SarahWeiii/CoACD#100) so users running into the misalignment know to set pca=False until a fix lands upstream. The flag still works the same way — this is a soft warning, not a hard assertion, since users may legitimately want to opt into the buggy behaviour for compatibility with old caches.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f06b7b5b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # decomposed convex hulls back, so the resulting collision body is misaligned with the | ||
| # original mesh. See https://github.com/SarahWeiii/CoACD/issues/100. | ||
| if self.pca: | ||
| gs.logger.warning( |
There was a problem hiding this comment.
CoacdOptions(pca=True) now calls gs.logger.warning(...) unconditionally, but gs.logger is initialized as None until gs.init() runs (see genesis/__init__.py), so constructing this options object before initialization will raise an AttributeError instead of emitting the intended warning. This is a regression for workflows that build options upfront and initialize Genesis later; the warning path should tolerate an uninitialized logger.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #2747. When
gs.options.CoacdOptions(pca=True)is used, upstream CoACD applies a PCA transformation to the input mesh but never transforms the decomposed convex hulls back. The result: collision bodies are misaligned with the original mesh, and stable scenes collapse on contact (see the screenshots in #2747).This is a known upstream issue: SarahWeiii/CoACD#100. Genesis cannot fix it without a CoACD release, but we can save users from silently hitting it.
Change
CoacdOptionsnow has a Pydanticmodel_validatorthat emits a clear warning whenpca=Trueis set, pointing at the upstream issue and suggestingpca=Falseas the workaround. Default-constructed options (pca=False) stay silent.I went with a warning rather than the hard
assert pca=Falsethe issue suggests because it preserves backward-compatible construction (users may want to opt into the buggy behaviour to keep an old hull cache stable, or run experiments with the upcoming CoACD fix), and matches the soft-warning convention used elsewhere ingenesis/options/(e.g. the deprecation warnings inmorphs.py).Diff
genesis/options/misc.py— addmodel_validator(mode="after")that callsgs.logger.warning(...)whenself.pcais True. Importsmodel_validator,Self, andgenesis as gs(the validator pattern already used bymorphs.py:106).tests/test_misc.py— two@pytest.mark.requiredunit tests:test_coacd_options_warn_when_pca_trueassertsgs.logger.warningis called once with"pca=True"and the upstream issue URL.test_coacd_options_no_warning_for_pca_false_defaultassertsCoacdOptions()andCoacdOptions(pca=False)are both silent.The mocking pattern matches
tests/test_utils.py::test_warn_once_logs_once.Test
Pre-fix repro: stash the
CoacdOptionsvalidator, run the new tests → both fail (no warning fires forpca=True). Restore the validator → 2/2 pass.ruff format --checkandruff checkclean on both touched files.I couldn't run the full
pytest -m requiredsuite locally because Genesis depends on a customquadrantspackage and CUDA-enabled torch that I don't have on this machine; the CI runner will catch any regression. The new tests are entirely about the option-level validator and don't touch the simulator stack.Notes
pca=Falseusers see no log noise.warn_oncehelper fromgenesis.utils.warningsto dedupe across manyCoacdOptions(pca=True)constructions, but that helper is not currently used ingenesis/options/and would silently suppress the warning across separate experiments. A per-construction warning matches Pydantic's per-instance validator semantics.Generated with Claude Code.