Skip to content

KnotHVP trait, field, added to DTO API; threaded through constructors#115

Merged
gennadiryan merged 4 commits into
mainfrom
feat/matrix-free-obj-constr
Jun 30, 2026
Merged

KnotHVP trait, field, added to DTO API; threaded through constructors#115
gennadiryan merged 4 commits into
mainfrom
feat/matrix-free-obj-constr

Conversation

@gennadiryan

Copy link
Copy Markdown
Member

DTO Issue #113 implementation summary (v1)

Status: Implemented and tested on 2026-06-30 against
DirectTrajOpt.jl@feat/matrix-free-obj-constr. Not yet committed.
Test log archived at ../../tmp/knot_hvp_run1.log.

Companion artifacts:
./dto-matrix-free.md — original design doc;
this summary covers Interface A (KnotHVP) only.


Test results

Test Pass Total
trait defaults to nothing for every objective 6 6
ConstantLowRankHVP round-trips via KnotPointObjective 3 3
CustomKnotHVP round-trips via KnotPointObjective 4 4
TerminalObjective threads knot_hvp keyword 2 2
default field value is nothing (no-regression smoke) 7 7
Total 22 22

Wall time 4.1s; no warnings, no errors.
17:41:42 [39/1898]

  • NEW src/objectives/knot_hvp.jlKnotHVP abstract,
    ConstantLowRankHVP(A::Matrix{Float64}, core::Symbol),
    CustomKnotHVP(apply!::Function, on_device::Bool), generic
    knot_hvp(::AbstractObjective, ::NamedTrajectory) = nothing, and the
    5 inline @testitem blocks.
  • src/objectives/_objectives.jl — exports of KnotHVP,
    ConstantLowRankHVP, CustomKnotHVP, knot_hvp (4 lines), and
    include("knot_hvp.jl") placed before knot_point_objectives.jl
    so KnotHVP is in scope when the struct's field type is parsed.
  • src/objectives/knot_point_objectives.jl — added
    knot_hvp::Union{Nothing,KnotHVP} field to the struct,
    knot_hvp=nothing keyword on the canonical outer constructor
    (Outer 1 at :68), threaded through to the inner. The other three
    outer constructors and both TerminalObjective variants forward
    kwargs... so they pick up the keyword automatically. Added the
    trait specialization
    knot_hvp(obj::KnotPointObjective, ::NamedTrajectory) = obj.knot_hvp
    next to Base.show. Docstring updated to replace the phantom
    ∂²Ls line (a doc artifact from the 6aeceee refactor — never an
    actual field) with the real knot_hvp field description.

Acceptance criteria (issue #113)

  • KnotHVP, ConstantLowRankHVP, CustomKnotHVP defined beside
    get_full_hessian.
  • Optional knot_hvp::Union{Nothing,KnotHVP} = nothing on
    KnotPointObjective; threaded through TerminalObjective via
    kwargs....
  • Trait with generic default + KnotPointObjective specialization.
  • Docstrings define the contract: data layout, core symbol
    convention, apply! signature, on_device host-staging semantics.
  • Default returns nothing for every current objective type (T1
    covers KnotPointObjective, QuadraticRegularizer,
    MinimumTimeObjective, GlobalObjective, CompositeObjective,
    NullObjective).
  • Minor release bump (v0.9.6 → v0.9.7 per semver, additive) —
    not done in this turn; flag for the commit step.

Out of scope (carried forward)

The remaining items from
./dto-matrix-free.md and
./piccolissimo-matrix-free.md remain
unstarted:

  1. Piccolissimo Issue #179 — consume knot_hvp(obj, traj) in the
    Altissimo backend; sunset PR #178's per-iterate probe+SVD for
    Tier-1 objectives. Blocked on this PR + compat bump.
  2. DTO StepHVP capability — sibling interface for the
    integrator-side constraint-curvature HVP. Independent of Add a KnotHVP capability interface: objectives can declare a matrix-free per-knot HVP #113;
    release-decoupled.
  3. Altissimo backend wiring
    AltissimoOptions.constraint_curvature_hvp flag + closure
    dispatched on step_hvp_capability.
  4. Phase 3 — GPU device-path constraint HVP — the big lift,
    in-scope per prior decision.

Full DTO no-regression sweep also deferred per prior
compute-conservation guidance.

@gennadiryan gennadiryan merged commit 7d67589 into main Jun 30, 2026
7 checks passed

@github-actions github-actions Bot 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.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'DirectTrajOpt.jl benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 0236c16 Previous: 0c47f87 Ratio
scaling_N25_d16_ipopt [alloc] 12424789480 bytes 2155582496 bytes 5.76
scaling_N25_d4_ipopt [wall] 1.38803937 s 1.00619233 s 1.38
scaling_N25_d4_ipopt [alloc] 1049401040 bytes 347578416 bytes 3.02
scaling_N51_d4_ipopt [wall] 1.972414571 s 0.597400298 s 3.30
scaling_N51_d4_ipopt [alloc] 3414859560 bytes 1253140304 bytes 2.73
evaluator_micro_bilinear_N51 / eval_hessian_lagrangian [median] 26243710 ns 21857630 ns 1.20

This comment was automatically generated by workflow using github-action-benchmark.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 35.29412% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/objectives/knot_hvp.jl 26.66% 11 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant