Repository navigation
Fix logically missing pieces in simulation and validation - #214
Conversation
- Fixed `to_mjcf` bug in `worker_heavy/utils/validation.py` to use MuJoCo backend and return XML. - Moved wire failure checks outside of `fem_enabled` block in `SimulationLoop`. - Implemented `MotorControlMode.ON_OFF` in `worker_heavy/utils/validation.py`. - Fixed `MuJoCoBackend.apply_jitter` to robustly find the free joint. - Made actuator types configurable in `MuJoCoSimulationBuilder`. - Added missing `zone_` body filtering in `SimulationLoop` to prevent unintended collision failures. - Fixed an unrelated failure in `tests/worker_heavy/test_node_validation.py` by adding required file to test data. Co-authored-by: MRiabov <108194191+MRiabov@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
This PR addresses several areas of logically missing or incorrect functionality in the simulation and validation logic:
to_mjcfUtility: Previously hardcoded to use the Genesis backend, which returned JSON instead of the MuJoCo XML format implied by its name and docstring. It now correctly uses the MuJoCo backend.SimulationLoop. They have been moved outside to ensure electronics are validated regardless of whether FEM is enabled for the parts.ON_OFFcontrol mode (present in the enum) was unhandled in the simulation loop. It now uses a frequency-based square-wave toggle logic.apply_jitterused hardcoded indicesqpos[0:3], which only worked if the target object was the first moving body. It now dynamically resolves the correct joint index for any body.MuJoCoSimulationBuilderpreviously hardcoded all actuators as "position" (servos). It now selects between "position", "velocity", and "motor" based on the part's control mode.SimulationLoopto exclude bodies starting withzone_from monitored collisions, preventing "forbid_zone_hit" failures caused by checking a site against its own parent body.tests/worker_heavy/test_node_validation.pyto includeassembly_definition.yaml, which is now a required file for the planner node.Verified with custom tests and the existing test suite.
PR created automatically by Jules for task 9186077024790278615 started by @MRiabov