refactor(solvers): consolidate local solver API, drop numba, prune dead code#11
Merged
Conversation
Squash of the review-driven correctness branch: - manifolds: correct 2D Lyapunov matrix in PSDFixedRank projection - sdp: repair broken entry points; SDP tests run without Mosek - utils: RobotURDF raises ValueError instead of returning string literals - solvers: LocalSolver honors all goals and uses the true SE2 left Jacobian; convex iteration no longer mutates the caller's anchors dict - BOUNDED edge data consistently checked via constants with .get() defaults across sdp_snl, joint_angle_solver, graph, dgp - docs: fix malformed ICRA BibTeX entry; add_anchor_node typo and triple-computed norm
Deletions (no callers anywhere in package, tests, or experiments;
verified by grep and a full-package import sweep):
- graphik/utils/operators.py (entire module) and the dgp.py helpers
that only it consumed (orthogonal_procrustes, factor, sample_matrix,
incidence_matrix_from_adjacency)
- graphik/utils/chordal.py: verbatim copy of networkx's
complete_to_chordal_graph; sdp_snl now calls nx's directly
- graphik/solvers/constraints.py: SOS-era sympy layer, broken since the
2D/3D unification (robot_graph.directed, parent_node_id). The one
live function, get_full_revolute_nearest_point, moved to sdp_snl.py
- utils.py: dZ, perpendicular_vector, norm_sq, list_to_variable_dict_-
spherical, variable_dict_to_list, make_save_string, spherical_angle_-
bounds_to_revolute, safe_arccos, bernoulli/wilson confidence helpers,
measure_perturbation, constraint_violations, __main__ block
- kinematics.py: fk_2d (only self-recursive)
- robot.py: end_effector_pos (iterated characters of a node name),
jacobian_geometric (referenced removed .parents), lambdified,
spherical
- graph.py: distance_bounds_from_sampling (dead, wrote garbage DIST)
- joint_angle_solver.py: gen_objective_ee/gen_grad_ee (superseded by
gen_cost_and_grad_ee), main() with ~60 lines of commented-out demo,
unused imports
- sdp_snl.py: sym_vec, constraints_list_to_matrix, ~130-line __main__
Mosek demo, unused typ assignment
- convex_iteration.py: random_psd_matrix, solve_fantope_sdp (+ its
only-callers fantope_constraints, solve_fantope_iterate),
get_sparsity_pattern, ~150-line __main__ benchmark
- robot_visualization.py: import-time rc('text', usetex=True) side
effect (broke import without LaTeX) is now an opt-in use_latex_text()
setup.py: sympy and progress dependencies dropped (only dead code used
them). Suite: 91 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The full suite now runs without Mosek (free-solver SDP tests with loose tolerances) and without the optional numba AOT extension, so a plain 'pip install -e .[dev]' is all CI needs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The EDM loss's Gauss-Newton Hessian has graph-Laplacian block structure (rank-1 edge blocks 8w_ij uu^T, u = y_i - y_j), which is both why tCG needed ~40 HVPs per outer iteration and why a near-exact preconditioner is cheap: at IK sizes the N*d x N*d GN matrix factors in microseconds. make_gn_preconditioner inverts it with relative eigenvalue flooring (default 1e-3, swept empirically; the GN matrix is PSD with a translation nullspace) and one eigh per outer iteration, memoized per Y; the output is projected back to the horizontal space. A block-Jacobi variant is included for comparison. UR10 with joint limits, 60 random goals over 3 seeds (see experiments/rtr_preconditioner_study.py): precon=None: 54/60 success, ~40 HVPs/outer, median 77-147 ms/solve precon='gn': 57/60 success, ~4 HVPs/outer, median 29-66 ms/solve Opt-in via solve(..., precon='gn'|'jacobi'|callable) and solve_with_riemannian(..., precon=...): preconditioning changes the optimization trajectory, so flipping the default belongs with a baseline regeneration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds init='zero' alongside the spectral and bound-smoothing initializations; exposed in rtr_profile for comparison.
The Gauss-Newton preconditioner removed the performance motivation for the AOT-compiled costgrd kernels; the dense NumPy loss backend (already the default and FD-verified) is now the only backend. - delete graphik/solvers/costs.py (numba.pycc AOT module) and the _numba_* wrappers in loss.py; drop the jit kwarg from the loss factories - hard-remove jit/use_jit from RiemannianSolver, NonlinearSolver and solve_with_riemannian; stale kwargs now raise TypeError instead of being silently swallowed by the **kwargs setattr loop - drop the njit branch from PSDFixedRank - drop numba/llvmlite from setup.py, the macOS lockfile, and the README AOT build instructions Default solver trajectories are unchanged (dense kernels untouched). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add IKSolver and IKResult, move shared EDM plumbing into DistanceSolver, rename loss.py to costs.py, single-source initialization strategies, and replace the old local solver modules with RiemannianSolver, ScipySolver, and JointAngleSolver on solve(T_goal). Migrate tests, baselines runner, benchmarks, examples, and README to the problem-level API. SDP solvers are unchanged.
Build L-BFGS-B position Bounds from the per-solve goal graph so POS-tagged end-effector goal nodes are hard-pinned instead of only represented by soft EDM residuals. Regenerate the solver-consolidation baseline for the intentional L-BFGS-B behavior change.
Simplification pass over the non-SDP codebase (sdp_snl, convex_iteration, and sdp_formulations are deliberately untouched): - graphs: extract _revolute_edge_bounds, the revolute reachable-distance core previously duplicated between _root_angle_limits_3d and _set_limits_3d; collapse check_distance_limits to one classification + two-sided check; O(1) node index in distance_bound_matrices; replace the star import with explicit imports; cached_property for node lists. - robots: drop the no-op kinematic_map property pair and the dead limited_joints property; cached_property for joint_ids/end_effectors/ T_base. - roboturdf: collapse the five identical load_* functions into load_urdf_robot + URDF_FILES (old names kept as thin wrappers); remove unused members (find_joint_by_name, find_joints_with_parent_link, joint_limits, n_urdf_joints, scene, urdf_ind_to_q); make get_parents and find_end_effector_joints side-effect free. - delete robot_visualization.py (unused; plot_3d_chain_manipulator called the nonexistent Robot.get_pose) and rtr's unused stopping-criterion label helpers. - move table_environment to graphik/utils/environments.py; give graphik.utils explicit re-exports with __all__. - packaging: migrate setup.py to pyproject.toml; export the public API (solvers, ProblemGraph, Robot) from the top-level graphik package. Verified behavior-preserving: full suite passes (123) and graph construction artifacts (distance/bound matrices, BOUNDED tags, limited_joints, violation reports) are bit-identical to the pre-refactor tree for UR10, KUKA, and a planar chain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Summary
Consolidates the local IK solvers behind a single problem-level API, removes the numba dependency, and cleans up dead code and duplication accumulated across the codebase. Net −619 lines across 63 files.
The branch builds in three phases:
Solver consolidation
IKSolver/IKResultbase with shared EDM plumbing inDistanceSolver; the old per-solver modules are replaced byRiemannianSolver,ScipySolver, andJointAngleSolver, all driven throughsolve(T_goal). Tests, baselines runner, benchmarks, examples, and README migrated to the new API.numba/llvmlitedropped from deps, lockfile, and README.load_urdf_robot, pruned dead code; pinned L-BFGS-B goal nodes.refactor(solvers)!: consolidate local solver API— old local-solver modules/entry points are replaced by the problem-levelsolve(T_goal)API. SDP solvers are unchanged.refactor(solvers)!: remove numba dependency—jit/use_jitkwargs are hard-removed; passing them now raisesTypeErrorinstead of being silently swallowed.Test plan
pytest— 123 passed (conda envgraphik_public), 0 deprecation warnings.🤖 Generated with Claude Code