Skip to content

Add per-point Yoshikawa manipulability to workspace analysis - #614

Merged
Yuan-Xinyi merged 10 commits into
mainfrom
xinyi/workspace01
Sep 15, 2026
Merged

Yuan-Xinyi merged 10 commits into
mainfrom
xinyi/workspace01

Conversation

@Yuan-Xinyi

Copy link
Copy Markdown
Collaborator

Stack

Description

This PR wires true per-point Yoshikawa manipulability into workspace analysis and documents a measured, configuration-only speedup of Cartesian/plane reachability analysis via the seed-selection sampler from #599.

1. Per-point manipulability (new)

The metrics/ subpackage was never invoked by the analyzer (_compute_metrics carried a TODO), and ManipulabilityMetric without Jacobians fell back to a centroid-distance placeholder. We measured that placeholder against ground truth on Franka (470 reachable points):

placeholder true Yoshikawa
mean 0.4196 0.0644
correlation with truth −0.366 1.0

The placeholder is not merely inaccurate — it is anti-correlated with true manipulability, so any consumer ranking by it preferred worse configurations. This PR:

  • computes w = sqrt(det(J J^T)) from the active solver's Jacobian over the stored joint_configurations after analysis (all three modes), row-aligned with the configurations and, in Cartesian/plane modes, with the reachable points;
  • stores manipulability_scores in the results dict and results.npz, restores it on cache hits, and reports aggregates under metrics["manipulability"] (closing the _compute_metrics TODO via the metrics module);
  • gates computation on MetricConfig.enabled_metrics (default ALL — on);
  • removes the placeholder: ManipulabilityMetric without Jacobians or precomputed scores now warns and returns no statistics — fabricated numbers are worse than none;
  • cost is negligible: batched Jacobian + determinant is ~10.5 ms for 470 configurations (GPU), well under 0.5% of the analysis.

Downstream, the aligned scores enable score-weighted runtime sampling (RobotWorkspace already supports weights) and manipulability-aware seed re-ranking as follow-ups; both are out of scope here.

2. Seed-selection speedup for Cartesian/plane analysis (documentation + measurement)

Cartesian/plane reachability runs through the solver's multi-start get_ik, where each analyzer seed occupies slot 0 of the solver-internal multi-start — so PytorchSolverCfg.enable_seed_selection (#599) applies with zero analyzer changes. Measured on Franka, 4000 identical sampled points (fixed RNG), warm timings (compile excluded), 1 seed/point:

solver config reachable detected wall time speedup
num_samples=30, random (default) 470 (11.8%) 3.3 s
enable_seed_selection, num_samples=30 475 (11.9%) 1.0 s 3.3×
enable_seed_selection, num_samples=8 466 (11.7%) 0.8 s 4.1×
enable_seed_selection, num_samples=4 460 (11.5%) 0.5 s 6.6×

At unchanged num_samples=30 the seeded configuration is strictly better: more reachable points detected and 3.3× faster (good seeds converge in fewer DLS iterations under early_stopping_any_converged). The recipe and numbers are recorded in the robot-workspace context docs; analytic solvers (OPW/SRS/UR) are unaffected.

Dependencies: none beyond #606.

Type of change

  • Enhancement (non-breaking change which improves an existing functionality)

Screenshots

N/A

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation (agent_context robot-workspace topic: manipulability contract, seed-selection recipe; MAP keywords)
  • Public API changes are reflected in the API docs (python docs/scripts/check_api_docs.py: 1853/1853)
  • I have added tests that prove my feature works (tests/sim/motion/workspace/test_manipulability.py: exact Yoshikawa on synthetic Jacobians, precomputed-score precedence, no-fabricated-statistics guard, end-to-end alignment on CobotMagic, metric gating, cache serialization round-trip)
  • Dependencies have been updated, if applicable (none)

Validation

pytest tests/sim/motion/workspace/test_manipulability.py  -> 7 passed
pytest tests/sim/motion/workspace/                        -> full suite (regression)
python docs/scripts/check_api_docs.py                     -> 1853/1853
black . / context.py check / agent-context map tests      -> clean

Note: one behavioural change is intentional — ManipulabilityMetric.compute() without Jacobians/scores now returns {} with a warning instead of placeholder statistics. Given the measured anti-correlation, silent consumers of the old numbers were being misled; failing loudly is the safer contract.

yuecideng and others added 2 commits September 10, 2026 08:07
Wire the metrics module into WorkspaceAnalyzer (closing the long-standing
_compute_metrics TODO) and compute true per-configuration Yoshikawa
manipulability w = sqrt(det(J J^T)) from the active solver's Jacobian after
every analysis mode. Scores are row-aligned with joint_configurations (and
with reachable points in Cartesian/plane modes), stored in results.npz,
restored on cache hits, and aggregated under metrics["manipulability"].
Computation is gated on MetricConfig.enabled_metrics and costs ~10 ms per
470 configurations on GPU.

Remove ManipulabilityMetric's centroid-distance placeholder: measured on
Franka it is negatively correlated with true manipulability (corr = -0.37),
so consumers ranking by it preferred worse configurations. Without
Jacobians or precomputed scores the metric now warns and returns no
statistics instead of fabricating them.

The batching test's mock robot now returns None from get_solver, faithful
to Robot.get_solver with no solvers attached.

Also documented in the robot-workspace context: enabling the #599
seed-selection sampler speeds Cartesian reachability analysis 3.3x at
unchanged num_samples=30 while detecting slightly more reachable points
(measured on Franka, 4000 identical targets), with no analyzer changes.

Covered by tests/sim/motion/workspace/test_manipulability.py; the full
workspace suite passes (68 passed).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Yuan-Xinyi Yuan-Xinyi added enhancement New feature or request robot Module related to robot labels Sep 11, 2026
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable new issue remains, and all previous findings are resolved in the current code.

Summary

This PR adds shared Yoshikawa manipulability primitives, integrates aligned per-configuration scores and aggregates into workspace analysis and caching, and supports manipulability-based selection among successful multi-start IK candidates. Follow-up changes also move preview joint-type inspection behind a backend-neutral articulation API.

  • Computes and persists per-configuration manipulability scores while repairing or stripping cached metric fields according to the current configuration.
  • Computes optional Jacobian condition statistics and removes the former Cartesian-distance placeholder.
  • Adds configurable manipulability-based multi-start IK candidate selection.
  • Adds API documentation, project context, tutorials, and focused numerical, solver, workspace, cache, and articulation tests.
  • The four previous cache and isotropy findings are manually resolved and are fully addressed by the current implementation.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Workspace analysis] --> B[Joint configurations]
    B --> C[Solver Jacobians]
    C --> D[Yoshikawa scores]
    C --> E[Condition numbers]
    D --> F[Aligned result arrays]
    E --> G[Metric aggregates]
    F --> H[Results cache]
    H --> I[Repair or strip on cache hit]

    J[Multi-start IK candidates] --> K[Successful candidates]
    K --> L{Selection mode}
    L -->|nearest| M[Nearest successful posture]
    L -->|manipulability| N[Highest Yoshikawa score]
Loading

Reviews (7) · Last reviewed commit: "Merge branch 'main' into xinyi/workspace..."

Comment thread embodichain/lab/sim/motion/workspace/analyzer.py Outdated
Comment thread embodichain/lab/sim/motion/workspace/analyzer.py Outdated
Address two review findings on the manipulability integration:

1. Metric settings are deliberately not part of the results-cache key (a
   metric toggle must not invalidate the expensive sampling/IK work), so a
   cache entry written under a different metric configuration — or before
   manipulability existed — could be returned without scores. The cache-hit
   path now runs the same _apply_manipulability step as fresh analysis,
   recomputing scores and aggregates from the cached joint configurations
   in milliseconds and repairing such entries transparently.

2. The analyzer reduced Jacobians to Yoshikawa scalars and discarded them,
   so the default compute_isotropy=True could never produce its documented
   condition statistics. The chunked Jacobian sweep now also collects
   condition numbers (max/min singular value) when isotropy is enabled, and
   ManipulabilityMetric accepts them precomputed.

Tests cover the repair path (an entry written with manipulability disabled
is repaired by a later default-enabled hit on the same key), isotropy
presence via the analyzer, and precomputed condition-number passthrough.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread embodichain/lab/sim/motion/workspace/analyzer.py
The repair-on-load path handled a disabled-producer entry hit by an
enabled run, but not the symmetric direction: an enabled-producer entry
hit by a disabled run leaked stale manipulability_scores and
metrics["manipulability"] into the returned results, diverging from the
fresh-analysis contract. _apply_manipulability now strips both fields
when the metric is disabled, and keeps cached scores when the metric is
enabled but locally not computable (they remain valid for the same joint
configurations).

Covered by test_cache_hit_strips_fields_when_metric_disabled, which
writes a score-bearing entry first so the strip path is genuinely
exercised on the hit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread embodichain/lab/sim/motion/workspace/analyzer.py Outdated
…config

On a cache hit where the current robot has no solver, the previous branch
kept both the cached scores and the producer's aggregate metrics, so the
returned means/counts could reflect a different jacobian_threshold and
condition statistics could be present with isotropy disabled (or stale
when enabled). Cached scores are pure kinematics and stay valid, but
aggregates now always go through ManipulabilityMetric under the CURRENT
configuration; per-point condition numbers are not cached, so condition
statistics are correctly absent on this path instead of leaking through.

Covered by test_no_solver_hit_recomputes_aggregates_under_current_config:
a mock no-solver robot hits an entry carrying producer aggregates from a
different configuration, and the returned aggregates honour the current
threshold while the stale mean_condition is removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Base automatically changed from perf/workspace-batching to main September 12, 2026 14:58

@yuecideng yuecideng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested

The core direction is useful, but I recommend addressing the following before merging.

1. [P2] Keep the public ManipulabilityMetric contract and its documentation consistent

manipulability_metric.py:90-100 now returns {} when neither Jacobians nor precomputed scores are supplied. However, the public metrics guide still documents ManipulabilityMetric().compute(workspace_points) as the basic usage and immediately indexes mean_manipulability (metrics.md:43-59). That documented example now raises KeyError.

Please either update the public documentation/examples to require Jacobians or precomputed scores, or provide an explicit opt-in heuristic mode with a clear deprecation path.

2. [P2] Report zero valid points correctly

The new analyzer path routes every computed score through _apply_manipulability. If a robot is at a singular posture, or the configured threshold is higher than every score, valid_scores becomes empty and is replaced with [0.0]. The result then reports num_valid_points == 1 although no point passed the threshold.

Please preserve a zero valid-point count and define an explicit empty-statistics behavior. Add a regression test with an all-zero or all-below-threshold score array.

3. Add a focused manipulability visualization

Please add a minimal workspace visualization path that colors reachable points/voxels by manipulability, keeps unreachable points visually distinct, uses robust/log normalization with a visible color bar, and exposes the raw score range. A selected point should be inspectable with its w, condition number, joint configuration, and (on demand) a translational manipulability ellipsoid. Avoid rendering an ellipsoid for every point; compute it for selected/top/bottom points only.

The visualization should preserve alignment for joint-space results and for reachable_points in Cartesian/plane results. Please add a focused example or test covering the mapping and normalization.

4. Add one application-oriented example

Please include a concise example showing a real decision driven by manipulability, rather than only printing aggregate statistics. The recommended first example is multi-seed IK re-ranking for one target pose: compare the first successful/nearest solution with the solution selected by manipulability, and show the resulting posture/ellipsoid and score.

For this to be meaningful, the candidate solutions must be scored before they are collapsed to the current “first successful seed” best_configs. A plane-sampling surface task (inspection, spraying, or polishing) would be a good second example if scope permits.

5. Extract the numerical calculation into a public compute module

The Jacobian-to-score calculation currently lives inside WorkspaceAnalyzer, which prevents IK solvers from reusing the same implementation to rank candidate solutions. Please move the pure, batched numerical helpers into a public module under the compute layer, e.g. embodichain.compute.kinematics.manipulability, and make both the analyzer and IK candidate-selection path call it.

The public API should cover at least:

  • Yoshikawa score sqrt(det(J @ J.T));
  • condition number / isotropy calculation;
  • batched Torch tensors with explicit dtype/device behavior;
  • optional translational or row-subset Jacobian selection for task-specific use.

Keep aggregation and workspace-specific result assembly in the workspace layer. Add focused tests for numerical correctness, singular/near-singular inputs, batched CPU execution, and deterministic candidate ranking. Export and document the new public API.

Yuan-Xinyi and others added 3 commits September 13, 2026 22:19
…unt report

Address review items R5, R2, and R1 on PR #614.

R5 — extract numerical calculation into a public compute module. Move the
Jacobian-to-score math out of WorkspaceAnalyzer into
embodichain.compute.kinematics.manipulability as pure batched Torch helpers:
- yoshikawa_manipulability(J) = sqrt(det(J @ J^T)), clamped at zero;
- condition_number(J) = sigma_max / sigma_min;
- select_jacobian_rows(J, "translational"|"rotational"|indices) for
  task-specific row subsets.
They preserve input dtype/device and never import simulation/workspace code.
The analyzer, the metric class, and future IK candidate-ranking now share
this single implementation; aggregation stays in the workspace layer.

R2 — report zero valid points correctly. When no score clears
jacobian_threshold (all singular, or threshold above every score),
ManipulabilityMetric previously substituted a single 0.0 score and reported
num_valid_points == 1. It now reports num_valid_points == 0 with NaN
statistics. Added regression tests for all-below-threshold and all-zero.

R1 — keep the public metric contract consistent. Updated the metrics guide:
its heuristic examples (which now raise KeyError) are replaced with
Jacobian-based usage, and the zero-valid-point / empty-return behaviour is
documented. Added the compute Kinematics API section and page.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address review items R5 (IK path must consume the shared compute helpers)
and R4 (application-oriented example) on PR #614.

PytorchSolver gains an opt-in ik_solution_selection="manipulability" mode.
The multi-seed pipeline previously always collapsed candidates to the one
nearest the caller seed; the new mode scores every successful candidate
with embodichain.compute.kinematics.yoshikawa_manipulability before the
collapse and keeps the best-conditioned posture. The default ("nearest")
is unchanged; invalid values are rejected at construction.

scripts/tutorials/sim/ik_manipulability_selection.py demonstrates the
decision on the DexforceW1 left arm (50 targets, 30 seeds each) and
separates the two effects: seed selection (iksel) improves which
candidates exist, re-ranking improves which candidate is kept.

  variant                  success  mean w   mean cond  time ms
  nearest (default)         100.0%  0.0125     165.2     180.6
  manipulability re-rank    100.0%  0.0162      75.2     176.9
  iksel + nearest           100.0%  0.0127     164.8     132.4
  iksel + manipulability    100.0%  0.0163      78.9     122.5

Re-ranking lifts mean manipulability +30% and halves the mean condition
number at no measurable cost; combined with iksel it keeps the +30% gain
at the lowest solve time. Focused tests cover mode validation, target
accuracy preservation, dominance over nearest under an identical
candidate pool, and deterministic selection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The compute-helper refactor dropped the pre-existing LinAlgError
degradation path. Keep the shared batched implementation as the primary
route, but on SVD failure fall back to per-matrix numpy computation with
inf on failure, exactly as before the refactor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Yuan-Xinyi

Copy link
Copy Markdown
Collaborator Author

Thanks for the detailed review — every point checked out. Five of the six items are addressed in the three new commits; the visualization (item 3) will follow as a separate PR.

1. Public metric contract (c94fb02) — the metrics guide examples now pass Jacobians (the old heuristic examples would raise KeyError), and the empty-return / zero-valid-point behaviour is documented. We did not add an opt-in heuristic mode: the removed centroid-distance placeholder measured negatively correlated with true manipulability (corr ≈ −0.37 on Franka), so there is no deprecation-worthy behaviour to preserve.

2. Zero valid points (c94fb02) — confirmed as a real bug: an empty valid set was replaced by [0.0] and reported num_valid_points == 1. It now reports num_valid_points == 0 with NaN statistics; regression tests cover all-below-threshold and all-zero score arrays. NaN round-trips safely through the results cache (Python JSON both ends).

3. Visualization — agreed on the full scope (manipulability-colored points with robust/log normalization and colorbar, distinct unreachable rendering, per-point inspection with w/condition/qpos, on-demand ellipsoids for selected/top/bottom only, joint-space and Cartesian alignment). It is the largest item, so it will come as a separate follow-up PR stacked on this one rather than growing this diff further.

4. Application example (8771437)scripts/tutorials/sim/ik_manipulability_selection.py runs the recommended multi-seed IK re-ranking on the DexforceW1 left arm (50 targets × 30 seeds, identical RNG so all variants rank the same candidate pool). As suggested, candidates are scored before the collapse to best_configs, via a new opt-in PytorchSolverCfg.ik_solution_selection="manipulability" (default "nearest" unchanged):

variant success mean w mean cond time
nearest (default) 100% 0.0125 165.2 180.6 ms
manipulability re-rank 100% 0.0162 (+30.1%) 75.2 176.9 ms
iksel + nearest 100% 0.0127 (+1.9%) 164.8 132.4 ms
iksel + manipulability 100% 0.0163 (+30.8%) 78.9 122.5 ms

Re-ranking lifts mean manipulability by ~30% and halves the mean condition number at no measurable cost. The iksel comparison separates the two effects cleanly: seed selection improves which candidates exist (speed), re-ranking improves which candidate is kept (posture quality) — they compose.

5. Compute extraction (c94fb02 + 8771437)embodichain.compute.kinematics.manipulability now provides yoshikawa_manipulability, condition_number, and select_jacobian_rows (translational/rotational/custom row subsets) as pure batched Torch helpers preserving dtype/device. The analyzer, the metric class, and the new IK candidate-selection path all call this single implementation. 14 focused numerical tests (closed-form checks, singular inputs yield 0/large-finite rather than NaN, batched CPU, deterministic ranking); exported and documented (API coverage gate 1859/1859).

8e5c276 restores the pre-existing per-matrix LinAlgError fallback in _compute_condition_numbers that the refactor had dropped — no unnecessary behaviour changes beyond what the review asked for. Existing workspace (14/14) and solver (14/14) suites pass unchanged.

This branch was stacked on #606, which main has since merged, so every
file #606 touched conflicted between that branch's original commit and
main's merged version.

Resolution rule: for files this branch never edited on top of #606
(base_solver.py, ur_solver.py, robot.py, test_analytic_batching.py,
the ik-solvers context page), take main's version wholesale — it is the
reviewed outcome of #606 plus later refactors, and keeping the older
pre-review copy would have reverted them. For files this branch does
own, keep its additions:

- compute/kinematics/__init__.py: main extended the kernel list with
  trapezoidal/Double-S profiles while this branch documented the
  manipulability helpers; both statements are kept.
- the robot-workspace context page and test_analysis_batching.py carry
  manipulability sections that main has no counterpart for.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Start a greploop in Codex and it will work through the open comments and keep going until this PR reviews clean.

@Yuan-Xinyi
Yuan-Xinyi merged commit 4694a8b into main Sep 15, 2026
1 check passed
@Yuan-Xinyi
Yuan-Xinyi deleted the xinyi/workspace01 branch September 15, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request robot Module related to robot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants