per-world meshes#1191
Conversation
|
Thanks for sharing - will try to dry-run this in newton. I think the direction looks good, we are using MjSpec. One thing that I'm failing to see on the first try is whether it's possible to control which world gets which randomization. I think that level of control would be useful, also for debugging. |
e6475dd to
a2c539c
Compare
a2c539c to
790b310
Compare
erikfrey
left a comment
There was a problem hiding this comment.
high level design: i really like this approach. it may be worth syncing with @adenzler-nvidia on whether we would want to specify a mapping per world explicitly, vs what you're doing here - i assume it's like a cartesian product or something?
my only suggestion is let's find some kind of naming convention in the custom tuples so it's obviously namespaced for this purpose. we may want to use custom tuples for other purposes too.
|
Unless it's technically impossible, I would vote for having the ability to specify the mapping explicitly. You never know what people want to be doing with this, and it seems like it makes sense to control this if needed. I didn't get to dry-run this yet because of release crunch but will do ASAP, hopefully still this week. |
790b310 to
41cd042
Compare
41cd042 to
25de185
Compare
|
@thowell if I am not mistaken, I think some properties e.g. masses are not updated correctly in some scenarios, here is an example: |
|
@jvonmuralt thank you for reporting this issue and providing a repro! the implementation was incorrect and should be fixed now. please let us know if the update does not resolve the reported issue, thanks! |
|
hello @thowell! I tried this somewhere and had these issues:
spec = mujoco.MjSpec.from_string(x)
mjm = spec.compile()
mjm.opt.integrator = mujoco.mjtIntegrator.mjINT_IMPLICITFAST
m = mjwarp.put_model(mjm)
m = per_world_mesh(m, spec, nworld=1)
self.assertEqual(m.opt.integrator, mujoco.mjtIntegrator.mjINT_IMPLICITFAST)also the fixes for preserving geom params wouldnt work if one is not unnamed per the def test_mesh_randomize_body_level_mass(self):
"""Body-level variant with fewer geoms must not include phantom mass.
vA: 2 unit cubes (±1), mass 8000 each, total 16000.
vB: 1 tiny cube (±0.1), mass 8.
Default body uses vA (max variant). vB disables 1 geom slot.
Disabled geom slots must not contribute to body_mass.
"""
_F = "0 2 1 0 3 2 4 5 6 4 6 7 0 1 5 0 5 4 2 3 7 2 7 6 0 4 7 0 7 3 1 2 6 1 6 5"
xml = f"""
<mujoco>
<asset>
<mesh name="a0" vertex="-1 -1 -1 1 -1 -1 1 1 -1 -1 1 -1 -1 -1 1 1 -1 1 1 1 1 -1 1 1" face="{_F}"/>
<mesh name="a1" vertex="-1 -1 -1 1 -1 -1 1 1 -1 -1 1 -1 -1 -1 1 1 -1 1 1 1 1 -1 1 1" face="{_F}"/>
<mesh name="b0" vertex="-.1 -.1 -.1 .1 -.1 -.1 .1 .1 -.1 -.1 .1 -.1 -.1 -.1 .1 .1 -.1 .1 .1 .1 .1 -.1 .1 .1" face="{_F}"/>
</asset>
<worldbody>
<body name="obj" pos="0 0 1">
<freejoint/>
<geom type="mesh" mesh="a0"/>
<geom name="g1" type="mesh" mesh="a1"/>
</body>
</worldbody>
<custom>
<tuple name="vA">
<element objtype="mesh" objname="a0"/>
<element objtype="mesh" objname="a1"/>
</tuple>
<tuple name="vB">
<element objtype="mesh" objname="b0"/>
</tuple>
<tuple name="obj">
<element objtype="tuple" objname="vA" prm="0.5"/>
<element objtype="tuple" objname="vB" prm="0.5"/>
</tuple>
</custom>
</mujoco>
"""
xml_b = f"""
<mujoco>
<asset>
<mesh name="b0" vertex="-.1 -.1 -.1 .1 -.1 -.1 .1 .1 -.1 -.1 .1 -.1 -.1 -.1 .1 .1 -.1 .1 .1 .1 .1 -.1 .1 .1" face="{_F}"/>
</asset>
<worldbody>
<body name="obj" pos="0 0 1">
<freejoint/>
<geom type="mesh" mesh="b0"/>
</body>
</worldbody>
</mujoco>
"""
expected_B = mujoco.MjSpec.from_string(xml_b).compile().body_mass[1]
nworld = 2
spec = mujoco.MjSpec.from_string(xml)
mjm = spec.compile()
m = mjwarp.put_model(mjm)
m = per_world_mesh(m, spec, nworld)
body_mass = m.body_mass.numpy()
dataid = m.geom_dataid.numpy()
for w in range(nworld):
if -1 in dataid[w]:
np.testing.assert_allclose(
body_mass[w, 1], expected_B, rtol=1e-4,
)thanks! |
61d3988 to
e1b9d26
Compare
|
@omarrayyann thanks for reporting these issues! the
please let us know if the updated implementation does not resolve the reported issues. thanks! |
| geom_enabled_idx = np.nonzero(geom_enabled_mask)[0] | ||
|
|
||
| mesh_registry = {} | ||
| mesh_bvh_id = [wp.uint64(0) for _ in range(nmesh)] |
There was a problem hiding this comment.
This was originally placed here to avoid building the mesh BVHs for collision meshes when rendering visual geoms, and vice versa. I think that is still relevant here.
|
@adenzler-nvidia are we good to move forward with this pr? thanks! |
|
sorry for the late reply - there is nothing from our side blocking this, we can use this well. That being said, for us it would make things much simpler to just get some of the mesh preprocessing functions exposed such that we could build the mesh array ourselves instead of going through put_model, and then assigning the dataids directly. This can/should be a follow-up though. |
|
@adenzler-nvidia the mesh preprocessing should all be available via |

update: the pr is now primarily about enabling batched
geom_dataid. see here for documentation (draft) containing examples utilizing this feature.per-world meshes
scene with 2 bodies
XML
Define via mjSpec