fix(dexforce_w1): merge PytorchSolver and SRSSolver defaults, fix solver_cfg merge#363
Merged
Merged
Conversation
…ver_cfg merge The _build_defaults() only set solver_cfg to SRSSolver entries for left_arm/right_arm, discarding PytorchSolver entries needed for whole-body IK. Now builds both families and merges with SRSSolver taking precedence for the 7-DoF arms. Also rewrites merge_robot_cfg's solver_cfg merge to use per-part logic instead of whole-dict replacement, fixing a bug where attribute-only overrides could silently drop other solver parts. Users can now reliably add custom control_parts and solver_cfg entries via init_dict. Fixes #120 Co-Authored-By: Claude <noreply@anthropic.com>
yuecideng
added
bug
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes DexforceW1’s default IK solver configuration so whole-body PytorchSolver entries are preserved alongside per-arm SRSSolver defaults, and it rewrites merge_robot_cfg’s solver_cfg merge logic to support safe per-part replacement vs. in-place attribute overrides.
Changes:
- DexforceW1 defaults now build PytorchSolver solver entries (including
full_body) and then overlay SRSSolver entries forleft_arm/right_arm. build_dexforce_w1_solver_cfg()now keys solver entries by control-part-aligned string names (and uses enum.valuewhere appropriate).merge_robot_cfg()now mergessolver_cfgper-part with explicit handling forclass_type(replace/add) vs. attribute-only overrides (in-place).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| embodichain/lab/sim/utility/cfg_utils.py | Reworks solver_cfg merging to be per-part and preserve unrelated solver entries. |
| embodichain/lab/sim/robots/dexforce_w1/utils.py | Aligns solver_cfg dict keys with control part names and fixes enum-key usage. |
| embodichain/lab/sim/robots/dexforce_w1/cfg.py | Builds merged default solver_cfg (PytorchSolver + SRSSolver overlay) instead of overwriting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
106
to
110
| provided_solver_cfg = override_cfg_dict.get("solver_cfg") | ||
| if provided_solver_cfg: | ||
| if provided_solver_cfg and isinstance(provided_solver_cfg, dict): | ||
| if base_cfg.solver_cfg is None: | ||
| base_cfg.solver_cfg = {} | ||
| for part, item in provided_solver_cfg.items(): |
Comment on lines
+133
to
+134
| if hasattr(target, attr_name): | ||
| setattr(target, attr_name, attr_val) |
Comment on lines
+130
to
+137
| pytorch_entries = build_dexforce_w1_solver_cfg( | ||
| arm_kind=self.arm_kind, | ||
| urdf_cfg=self.urdf_cfg, | ||
| ) | ||
| srs_entries = self._build_default_solver_cfg(arm_kind=self.arm_kind) | ||
| # Merge: SRSSolver takes precedence for arm keys | ||
| pytorch_entries.update(srs_entries) | ||
| self.solver_cfg = pytorch_entries |
Comment on lines
+111
to
+121
| if isinstance(item, dict) and "class_type" in item: | ||
| # New or replacement solver part — use the deserialized | ||
| # SolverCfg object produced by RobotCfg.from_dict. | ||
| parsed = ( | ||
| robot_cfg.solver_cfg.get(part) | ||
| if isinstance(robot_cfg.solver_cfg, dict) | ||
| else None | ||
| ) | ||
| if parsed is not None: | ||
| base_cfg.solver_cfg[part] = parsed | ||
| else: |
…and-dexforce-w1-defaults
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the default
solver_cfgconstruction for DexforceW1 and rewrites themerge_robot_cfgsolver merge to support general custom control-part + solver addition.Problem
DexforceW1
_build_defaults()only produced SRSSolverCfg entries forleft_arm/right_arm, discarding the PytorchSolverCfg entries needed for whole-body IK. Thebuild_dexforce_w1_solver_cfg()utility existed but was never called.merge_robot_cfghad broken solver merging: when a user passedsolver_cfgwithoutclass_type(attribute-only overrides like{"tcp": ...}), the old code replaced the entirebase_cfg.solver_cfgdict — silently dropping other solver parts. The loop mixed per-part and whole-dict logic in the same iteration.No clean mechanism for adding custom control parts + solvers: users could pass
control_partsandsolver_cfgviainit_dict, but the merge was fragile and undocumented.Changes
embodichain/lab/sim/robots/dexforce_w1/cfg.py—_build_defaults()now callsbuild_dexforce_w1_solver_cfg()for PytorchSolver entries (full_body), then overlays SRSSolver entries viadict.update(). SRSSolver takes precedence for arm keys (DH-based, more precise for 7-DoF arms).embodichain/lab/sim/robots/dexforce_w1/utils.py—build_dexforce_w1_solver_cfg():"left_arm","right_arm") instead of type-aligned keys ("left_arm1","left_arm2") that never matched any control part.valueusage for dict keys (raw enum members break regex matching ininit_solver)Dict[str, SolverCfg]embodichain/lab/sim/utility/cfg_utils.py— Rewrotemerge_robot_cfgsolver_cfg merge with three clean per-part modes:class_typepresent → new/replacement solver (deserialized viaRobotCfg.from_dict)class_type, part exists → merge individual attribute overrides in-place (preserves other parts)class_type, part unknown → warn and skipResult
Default DexforceW1 solver_cfg now has all needed entries:
left_arm→ SRSSolverCfg (DH-based, precise)right_arm→ SRSSolverCfg (DH-based, precise)full_body→ PytorchSolverCfg (URDF-based, whole-body IK) — NEWUsers can now reliably add custom control parts + solvers:
Fixes #120
Type of change
Checklist
black .command to format the code base.