Skip to content

Commit 46b90af

Browse files
Ruben Grandiacursoragent
andcommitted
Address Kamino solver configuration feedback
Document the public base configuration, keep its test coverage extensible, and provide a clear error when it is used without a concrete dynamics solver. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 4b47419 commit 46b90af

4 files changed

Lines changed: 22 additions & 5 deletions

File tree

docs/source/api/lab_newton/isaaclab_newton.physics.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
KaminoMaterialsCfg
3030
KaminoPADMMCfg
3131
KaminoPADMMSolverCfg
32+
KaminoSolverCfgBase
3233
MPMSolverCfg
3334
HydroelasticSDFCfg
3435

@@ -105,6 +106,11 @@ Physics Configuration
105106
:show-inheritance:
106107
:exclude-members: __init__
107108

109+
.. autoclass:: KaminoSolverCfgBase
110+
:members:
111+
:show-inheritance:
112+
:exclude-members: __init__
113+
108114
.. autoclass:: KaminoPADMMSolverCfg
109115
:members:
110116
:show-inheritance:

docs/source/overview/core-concepts/physical-backends/newton/kamino-solver.rst

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,8 @@ Kamino Solver Parameters
180180
------------------------
181181

182182
The following fields are shared by
183-
:class:`~isaaclab_newton.physics.KaminoPADMMSolverCfg` and
184-
:class:`~isaaclab_newton.physics.KaminoDVISolverCfg`. They are grouped by the part
185-
of the solver they affect.
183+
:class:`~isaaclab_newton.physics.KaminoSolverCfgBase`. They are grouped by the part of
184+
the solver they affect.
186185

187186
Core Integration
188187
^^^^^^^^^^^^^^^^

source/isaaclab_newton/isaaclab_newton/physics/kamino_manager_cfg.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,9 @@ class KaminoSolverCfgBase(NewtonSolverCfg):
329329

330330
def _get_dynamics_solver_config(self) -> tuple[Literal["padmm", "dvi"], dict[str, Any]]:
331331
"""Return the selected Newton solver name and its configuration keyword arguments."""
332-
raise NotImplementedError
332+
raise NotImplementedError(
333+
f"{type(self).__name__} is a base configuration. Use KaminoPADMMSolverCfg or KaminoDVISolverCfg."
334+
)
333335

334336
def to_solver_config(self) -> SolverKamino.Config:
335337
"""Build a :class:`SolverKamino.Config` from this configuration.

source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
KaminoDynamicsCfg,
3939
KaminoPADMMCfg,
4040
KaminoPADMMSolverCfg,
41+
KaminoSolverCfgBase,
4142
MJWarpSolverCfg,
4243
MPMSolverCfg,
4344
NewtonCfg,
@@ -163,6 +164,15 @@ def test_newton_cfg_post_init_propagates_class_type(
163164
assert cfg.class_type.__name__ == expected_manager.__name__
164165

165166

167+
def test_kamino_solver_cfg_base_requires_concrete_configuration():
168+
"""The public Kamino base config should direct users to a concrete solver config."""
169+
with pytest.raises(
170+
NotImplementedError,
171+
match="KaminoSolverCfgBase is a base configuration. Use KaminoPADMMSolverCfg or KaminoDVISolverCfg.",
172+
):
173+
KaminoSolverCfgBase().to_solver_config()
174+
175+
166176
@pytest.mark.parametrize(
167177
"num_substeps, collision_decimation, should_warn",
168178
[
@@ -1000,7 +1010,7 @@ def test_initialize_solver_populates_canonical_state(
10001010
# something to work with.
10011011
body = builder.add_body(mass=1.0)
10021012
builder.add_joint_revolute(parent=-1, child=body, axis=(0, 0, 1))
1003-
if isinstance(solver_cfg, (KaminoPADMMSolverCfg, KaminoDVISolverCfg)) and solver_cfg.use_collision_detector:
1013+
if isinstance(solver_cfg, KaminoSolverCfgBase) and solver_cfg.use_collision_detector:
10041014
builder.add_shape_sphere(body=body, radius=0.05)
10051015
builder.add_ground_plane()
10061016
NewtonManager.set_builder(builder)

0 commit comments

Comments
 (0)