Repository navigation
fix(conversions): cast geom_type to int in merge_geoms enum check - #32
Open
Guozhongyuan wants to merge 1 commit into
Open
Guozhongyuan wants to merge 1 commit into
Guozhongyuan wants to merge 1 commit into
Conversation
MjModel.geom_type[i] is a numpy scalar; tuple membership evaluates enum == np_scalar (element on the left), and MuJoCo's pybind11 enum returns False against numpy scalars. Mesh/SDF geoms therefore silently took the primitive path and raised 'Unsupported shape type: 7'. Match the int() idiom already used in merge_geoms_hull().
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.
Problem
merge_geoms()classifies geoms with:MjModel.geom_type[i]is a NumPy scalar (np.int32). Tuple membership evaluatesthe element on the left (
enum_member == np_scalar), and MuJoCo's pybind11 enumreturns False when compared against a NumPy scalar from that side — even though
the identical comparison with the NumPy scalar on the left is
True:So the membership test silently fails for every geom type. Mesh and SDF geoms
fall into the
create_primitive_mesh()branch and raise:Primitive geoms keep working only because the primitive fallback happens to
reconstruct the same shape. Any model with a mesh geom among its fixed-body
geometries — i.e. almost every real model — crashes when its static geoms are
merged. This also means the SDF routing added in #31 never actually triggers.
merge_geoms_hull()is unaffected because it already compares viaint(...),which is also why this slipped through CI: the existing
test_merge_geomsonlymerges primitive geoms.
Fix
Cast both sides to
int()before the membership test, matching the existingidiom in
merge_geoms_hull(). One-line change plus a regression test.Testing
test_merge_geoms_with_mesh_geom, which merges amjGEOM_MESHgeom:raises
ValueError: Unsupported shape type: 7before the fix, passes after(verified red/green).
Reproduced on mujoco 3.14.0, numpy 2.5.3, Python 3.12. The regression was
introduced by #31 (post-v0.0.14, main only), which is why released wheels are
unaffected.