Add per-point Yoshikawa manipulability to workspace analysis - #614
Yuan-Xinyi wants to merge 8 commits into
Conversation
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>
|
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>
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>
…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>
yuecideng
left a comment
There was a problem hiding this comment.
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.
…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>
|
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 2. Zero valid points (c94fb02) — confirmed as a real bug: an empty valid set was replaced by 3. Visualization — agreed on the full scope (manipulability-colored points with robust/log normalization and colorbar, distinct unreachable rendering, per-point inspection with 4. Application example (8771437) —
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) — 8e5c276 restores the pre-existing per-matrix |
Stack
perf/workspace-batchingDescription
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_metricscarried aTODO), andManipulabilityMetricwithout Jacobians fell back to a centroid-distance placeholder. We measured that placeholder against ground truth on Franka (470 reachable points):The placeholder is not merely inaccurate — it is anti-correlated with true manipulability, so any consumer ranking by it preferred worse configurations. This PR:
w = sqrt(det(J J^T))from the active solver's Jacobian over the storedjoint_configurationsafter analysis (all three modes), row-aligned with the configurations and, in Cartesian/plane modes, with the reachable points;manipulability_scoresin the results dict andresults.npz, restores it on cache hits, and reports aggregates undermetrics["manipulability"](closing the_compute_metricsTODO via the metrics module);MetricConfig.enabled_metrics(defaultALL— on);ManipulabilityMetricwithout Jacobians or precomputed scores now warns and returns no statistics — fabricated numbers are worse than none;Downstream, the aligned scores enable score-weighted runtime sampling (
RobotWorkspacealready 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 — soPytorchSolverCfg.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:num_samples=30, random (default)enable_seed_selection,num_samples=30enable_seed_selection,num_samples=8enable_seed_selection,num_samples=4At unchanged
num_samples=30the seeded configuration is strictly better: more reachable points detected and 3.3× faster (good seeds converge in fewer DLS iterations underearly_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
Screenshots
N/A
Checklist
black .command to format the code base.agent_contextrobot-workspace topic: manipulability contract, seed-selection recipe; MAP keywords)python docs/scripts/check_api_docs.py: 1853/1853)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)Validation
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.