Enhance collision scaling test for exported model #464
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
| Benchmark suite | Current: 128a9ac | Previous: cb36385 | Ratio |
|---|---|---|---|
tests/test_benchmark.py::test_forward_dynamics_aba[1] |
385.92369416125706 iter/sec (stddev: 0.00001015286825073992) |
386.2552251710958 iter/sec (stddev: 0.00020833313469613508) |
1.00 |
tests/test_benchmark.py::test_forward_dynamics_aba[128] |
28.93699332834927 iter/sec (stddev: 0.0001408388654496315) |
25.623251572538486 iter/sec (stddev: 0.000628276488211365) |
0.89 |
tests/test_benchmark.py::test_free_floating_bias_forces[1] |
359.7554458207316 iter/sec (stddev: 0.000008276072228480121) |
329.6208935032452 iter/sec (stddev: 0.00017678390636095912) |
0.92 |
tests/test_benchmark.py::test_free_floating_bias_forces[128] |
16.113562351572337 iter/sec (stddev: 0.00021695789656827282) |
14.042023282252773 iter/sec (stddev: 0.0004701444679618353) |
0.87 |
tests/test_benchmark.py::test_forward_kinematics[1] |
451.84797452037225 iter/sec (stddev: 0.000007592159697414122) |
505.33330250290584 iter/sec (stddev: 0.00003595675660629741) |
1.12 |
tests/test_benchmark.py::test_forward_kinematics[128] |
31.558336722612147 iter/sec (stddev: 0.00012441931661127614) |
29.344980191633002 iter/sec (stddev: 0.00036979712165659257) |
0.93 |
tests/test_benchmark.py::test_free_floating_mass_matrix[1] |
183.73411846012678 iter/sec (stddev: 0.00005627389138348078) |
204.56693174871816 iter/sec (stddev: 0.0008038774133928276) |
1.11 |
tests/test_benchmark.py::test_free_floating_mass_matrix[128] |
175.40337759179477 iter/sec (stddev: 0.000018251923053422307) |
185.32420162463208 iter/sec (stddev: 0.001038192142422702) |
1.06 |
tests/test_benchmark.py::test_free_floating_jacobian[1] |
523.2298479916684 iter/sec (stddev: 0.0000074579082376539724) |
584.370298139834 iter/sec (stddev: 0.00001860199318220017) |
1.12 |
tests/test_benchmark.py::test_free_floating_jacobian[128] |
528.89832863096 iter/sec (stddev: 0.000006251194657720329) |
544.3999181675149 iter/sec (stddev: 0.00023044493810550008) |
1.03 |
tests/test_benchmark.py::test_free_floating_jacobian_derivative[1] |
413.2146335693067 iter/sec (stddev: 0.000010489133865941805) |
410.73560028011394 iter/sec (stddev: 0.00022687143203532596) |
0.99 |
tests/test_benchmark.py::test_free_floating_jacobian_derivative[128] |
302.50305627956345 iter/sec (stddev: 0.000008816757160904768) |
199.38111540598362 iter/sec (stddev: 0.001457819036270435) |
0.66 |
tests/test_benchmark.py::test_soft_contact_model[1] |
351.4689243184375 iter/sec (stddev: 0.00000883049911085973) |
337.6366937284971 iter/sec (stddev: 0.00011127692295687577) |
0.96 |
tests/test_benchmark.py::test_soft_contact_model[128] |
29.775469666412476 iter/sec (stddev: 0.0001137212967783679) |
26.884777773429075 iter/sec (stddev: 0.0006199761164350777) |
0.90 |
tests/test_benchmark.py::test_rigid_contact_model[1] |
40.43216684781911 iter/sec (stddev: 0.000020991654619197454) |
31.251271243159007 iter/sec (stddev: 0.007996216837436577) |
0.77 |
tests/test_benchmark.py::test_rigid_contact_model[128] |
0.7258049538212741 iter/sec (stddev: 0.00036055551280812976) |
0.3460028427282277 iter/sec (stddev: 0.0005242995729556135) |
0.48 |
tests/test_benchmark.py::test_relaxed_rigid_contact_model[1] |
74.56232598760404 iter/sec (stddev: 0.00003041059627728627) |
79.40038194340299 iter/sec (stddev: 0.002071043703591531) |
1.06 |
tests/test_benchmark.py::test_relaxed_rigid_contact_model[128] |
6.371228238124662 iter/sec (stddev: 0.00013434061539048384) |
6.11243560362339 iter/sec (stddev: 0.0013425887323580527) |
0.96 |
tests/test_benchmark.py::test_simulation_step[1] |
69.60528015015585 iter/sec (stddev: 0.000039938666527643756) |
69.93411634859267 iter/sec (stddev: 0.0027247172648478526) |
1.00 |
tests/test_benchmark.py::test_simulation_step[128] |
5.383924738907867 iter/sec (stddev: 0.0003715633996499673) |
5.076712249345317 iter/sec (stddev: 0.00163782888871275) |
0.94 |
This comment was automatically generated by workflow using github-action-benchmark.
| assert isinstance(exported_urdf, str), "Exported model URDF is not a string." | ||
|
|
||
| # Load the exported URDF using ROD | ||
| exported_sdf = rod.Sdf.load(exported_urdf, is_urdf=True) |
There was a problem hiding this comment.
I am a bit confused about what this test do:
- creates an updated model where we scale some quantities
- exported it into a urdf
- check vs another model ?
could you please clarify ?
I think that, we may want to do two things:
- Validate the export model: for this it is sufficient and sound to introduce a scaling factor of 1, and re-export. you should obtain the same urdf /sdf
- Validate the scaling: Take a model, scale it via JAXSIM api, then comparing against a model (urd/sdf) which has been scaled manually and we know it is correct.
Seems to me this test is somewhere in between the two but maybe I am missing something
There was a problem hiding this comment.
I am a bit confused about what this test do:
* creates an updated model where we scale some quantities * exported it into a urdf * check vs another model ? could you please clarify ?
In the test we have two models:
- a base "garpez" ROD model
- an already scaled ROD model, for which we are sure about the final geometries, by construction
Then we:
- Scale the base the base model and export it as a URDF string (
exported_urdf) - Create the ROD model from that URDF (
exported_urdf) - Check that geometries, mass, inertias and collision shapes match between the original ROD model (the pre-scaled one), and the new ROD model
Validate the export model: for this it is sufficient and sound to introduce a scaling factor of 1, and re-export. you should obtain the same urdf /sdf
I'll add this
Validate the scaling: Take a model, scale it via JAXSIM api, then comparing against a model (urd/sdf) which has been scaled manually and we know it is correct.
This should be exactly what is happening in this test, if I understood you correctly
There was a problem hiding this comment.
I think that we are doing two checks in one , thus
- export model
- scaling
I woudl separate those, having the export checked against all ones. In this way in the future should be easier to debug things
Indeed here
Validate the scaling: Take a model, scale it via JAXSIM api, then comparing against a model (urd/sdf) which has been scaled manually and we know it is correct.
You are exporting the scale model to a urdf . I would check against jaxsim model, without exporting. So that we can isolate in the future if the problem is in the exporting or in the scaling
There was a problem hiding this comment.
I woudl separate those, having the export checked against all ones. In this way in the future should be easier to debug things
We already test those separately, namely "scaling" in test_update_hw_link_parameters and "exporting" in test_export_updated_model. We do not save any high level information on the collision shape inside JaxSim, but rather only the collidable points position. We might want to check those instead of comparing the ROD models, what do you think?
There was a problem hiding this comment.
Added scaling test with unitary scaling factor in 4eb74c6
There was a problem hiding this comment.
Added test to compare collidable points position directly from the JaxSim model in 128a9ac
There was a problem hiding this comment.
Yes I agree, we have two separate tests: one to check only the hw metadata after an update and test_export_updated_model to check the exported model against a known pre-scaled model
flferretti
force-pushed
the
add_collision_parametrization_test
branch
from
ee789fc to
128a9ac
Compare
This PR enhances the testing of exported ROD models by adding collision scaling validation and refactoring the existing export test for clarity.
Fix #458
📚 Documentation preview 📚: https://jaxsim--464.org.readthedocs.build//464/