-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[BUG FIX] Apply MeshSet poses to sampled particles. #3171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,37 @@ | ||
| import numpy as np | ||
| import pytest | ||
| import torch | ||
| import trimesh | ||
|
|
||
| import genesis as gs | ||
|
|
||
|
|
||
| @pytest.mark.required | ||
| def test_mesh_set_particle_transform(show_viewer): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please fold this MeshSet entity and its assertions into an existing MPM test and reuse that scene build. A separate test incurs another full Genesis initialization for every CI configuration. |
||
| pos = np.array((0.2, 0.1, 0.2)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please keep this value as a tuple: pos = (0.2, 0.1, 0.2)The scene-construction guidelines prohibit wrapping controlled array-like inputs in |
||
| mesh = trimesh.creation.box(extents=(0.04, 0.12, 0.04)) | ||
| scene = gs.Scene( | ||
| mpm_options=gs.options.MPMOptions( | ||
| lower_bound=(-0.5, -0.5, -0.5), | ||
| upper_bound=(0.5, 0.5, 0.5), | ||
| particle_size=0.01, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keyword arguments must follow the callee's declaration order, so The custom bounds are also unnecessary for this geometry because it already fits inside the default MPM domain. This can be reduced to: mpm_options=gs.options.MPMOptions(
particle_size=0.01,
), |
||
| ), | ||
| show_viewer=show_viewer, | ||
| ) | ||
| entity = scene.add_entity( | ||
| morph=gs.morphs.MeshSet( | ||
| files=(mesh,), | ||
| poss=(pos,), | ||
| eulers=((0.0, 0.0, 90.0),), | ||
| ), | ||
| material=gs.materials.MPM.Elastic(sampler="regular"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scene-building calls require one option per line, even when only one option is provided: material=gs.materials.MPM.Elastic(
sampler="regular",
), |
||
| ) | ||
|
|
||
| particles = entity.init_particles | ||
| np.testing.assert_allclose(particles.mean(axis=0), pos, atol=1e-6) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use the repository assertion utility instead of NumPy's testing API: from ..utils.assertions import assert_allcloseThen call An explicit |
||
| np.testing.assert_allclose(np.ptp(particles, axis=0), (0.11, 0.03, 0.03), atol=1e-6) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This catches a completely missing rotation, but it does not fully validate MeshSet pose semantics. A single centered box cannot detect applying the wrong member's pose, and +90° and −90° rotations have identical means and extents. Please test at least two members with different rotation axes and make one mesh off-center or assert another direction-sensitive quantity so an inverse rotation fails. Please also derive the expected spans from Note this interacts with the bounds comment above: keep every member inside the default MPM domain (z well above 0, within the safety padding), or the custom bounds become necessary again. |
||
|
|
||
|
|
||
| @pytest.mark.required | ||
| def test_particle_constraints(show_viewer): | ||
| scene = gs.Scene( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transform is mathematically correct, but please compute
pos_iandquat_ionce and use the same values for both the particles andself._vmesh[i]in this loop, then remove the later visual-transform loop. This avoids maintaining two derivations of the same member pose and keeps particle placement, rendering, and skinning in the same frame.These are validated internal tuples, so the repository guidelines also require
np.arrayrather thannp.asarray.