draft: refactor spawn - #516
Conversation
Greptile SummaryThe PR refactors scene spawning into declaration, materialization, runtime preparation, and facade-binding phases.
Confidence Score: 2/5The PR does not appear safe to merge because Spawn attribute configuration remains incomplete and failed facade binding still cannot be retried. Spawn-bound rigid objects still route friction access to unsupported methods, Spawn articulations still omit configured non-mass physics overrides, and a binding exception after runtime preparation still causes later prepare calls to return without retrying the failed bind. Files Needing Attention: embodichain/lab/sim/objects/backends/spawn.py, embodichain/lab/sim/objects/articulation.py, embodichain/lab/sim/sim_manager.py
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/sim_manager.py | Centralizes scene materialization, runtime preparation, and facade binding in SimulationManager. |
| embodichain/lab/sim/objects/articulation.py | Adds declared and Spawn-bound articulation initialization and configuration paths. |
| embodichain/lab/sim/objects/backends/spawn.py | Implements Spawn-backed rigid-body and articulation views. |
| embodichain/lab/gym/envs/base_env.py | Splits environment initialization into pre-prepare declaration and post-prepare configuration phases. |
| tests/sim/test_sim_manager.py | Extends SimulationManager preparation and configuration coverage. |
Reviews (3): Last reviewed commit: "wip" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def fetch_friction( | ||
| self, data: torch.Tensor, body_ids: torch.Tensor | None = None | ||
| ) -> None: | ||
| del data, body_ids | ||
| self._unsupported_property("friction") | ||
|
|
||
| def apply_friction(self, data: torch.Tensor, body_ids: torch.Tensor) -> None: | ||
| del data, body_ids | ||
| self._unsupported_property("friction") |
There was a problem hiding this comment.
When a caller invokes set_friction or get_friction on a Spawn-bound rigid object, the ready body view dispatches to methods that unconditionally raise NotImplementedError, causing existing rigid-object attribute operations to fail. The finalized Newton attribute path also invokes the unsupported friction, restitution, and contact-offset setters.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/lab/sim/objects/backends/spawn.py
Line: 280-288
Comment:
**Ready view exposes unsupported attributes**
When a caller invokes `set_friction` or `get_friction` on a Spawn-bound rigid object, the ready body view dispatches to methods that unconditionally raise `NotImplementedError`, causing existing rigid-object attribute operations to fail. The finalized Newton attribute path also invokes the unsupported friction, restitution, and contact-offset setters.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| if overrides.body_attributes is not None: | ||
| todos.append( | ||
| "non-mass articulation link physics attributes are retained in " | ||
| "cfg but DexSim SpawnedArticulation has no live common setter" | ||
| ) |
There was a problem hiding this comment.
When an ArticulationCfg defines non-mass body or per-link physics attributes, Spawn binding retains but never applies those values, causing the articulation to run with backend defaults instead of its configured friction, restitution, damping, contact, CCD, velocity, or solver behavior.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/lab/sim/objects/articulation.py
Line: 734-738
Comment:
**Articulation physics overrides are discarded**
When an `ArticulationCfg` defines non-mass body or per-link physics attributes, Spawn binding retains but never applies those values, causing the articulation to run with backend defaults instead of its configured friction, restitution, damping, contact, CCD, velocity, or solver behavior.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
black .command to format the code base.