Skip to content

fix(problems): default Δt bounds window + three-mode free-time guidance - #145

Draft
aarontrowbridge wants to merge 2 commits into
mainfrom
fix/freetime-default-bounds-warning-text
Draft

fix(problems): default Δt bounds window + three-mode free-time guidance#145
aarontrowbridge wants to merge 2 commits into
mainfrom
fix/freetime-default-bounds-warning-text

Conversation

@aarontrowbridge

Copy link
Copy Markdown
Member

Closes the DirectTrajOpt half of harmoniqs/Piccolo.jl#317 (free-time semantics).

What

  • Unbounded per-knot Δt now receives a default window of [nominal/4, nominal×4] instead of only a zero lower bound — unbounded free time with no duration anchor is ill-posed (measured: duration runaway to ~^5\times$ nominal, fidelity collapse on a drift-dominated 1-qubit benchmark).
  • The injected-bounds warning states the three free-time modes explicitly (uniform mesh with free duration / bounded per-knot / pinned) and no longer recommends the all-equal option as 'fixing' timesteps — it leaves one free scale degree of freedom by design.
  • Tests: default-window injection, explicit-bounds-wins, global-components variant (15/15 green on the touched items).

Follow-on (research track, not here): derived timesteps — remove Δt from the decision vector entirely; the same benchmark stalls at F=0.65 under pinned-but-variable timesteps where a derived-timestep prototype solves to ^{-12}$.

Re-vet flag: produced templates using unbounded free time will see bounded durations; per #317.

Unbounded per-knot free time is ill-posed — the optimizer spends the
timestep freedom on the discretization error itself (measured: duration
runaway and fidelity collapse on drift-dominated problems). When :Δt
carries no bounds, apply a default window of a quarter to four times the
nominal step instead of only clamping at zero; explicit bounds still win.

The injected-bounds warning text now states the three free-time modes
(uniform mesh with free duration / bounded per-knot / pinned) and no
longer mislabels the all-equal option as fixing timesteps — it leaves
one free scale degree of freedom by design.

Evidence and semantics survey: harmoniqs/Piccolo.jl#317

@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: 9fe75cd Previous: 110708d Ratio
scaling_N101_d16_ipopt [wall] 52.392247318 s 7.520233163 s 6.97
scaling_N101_d16_ipopt [alloc] 214081178896 bytes 31020145144 bytes 6.90
scaling_N101_d16_ipopt [iters] 50 iterations 6 iterations 8.33
scaling_N25_d8_ipopt [wall] 0.035958695 s 0.007596808 s 4.73
scaling_N25_d8_ipopt [alloc] 165824528 bytes 22601216 bytes 7.34
scaling_N51_d16_ipopt [wall] 7.255641397 s 1.104382817 s 6.57
scaling_N51_d16_ipopt [alloc] 28280315880 bytes 4518562000 bytes 6.26
scaling_N51_d16_ipopt [iters] 12 iterations 1 iterations 12

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

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@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 convergence'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 9fe75cd Previous: 082c146 Ratio
xgate_convergence_ipopt_N51 [wall] 20.195453639 s 16.579704121 s 1.22
xgate_convergence_madnlp_N51 [wall] 19.447817476 s 15.358710286 s 1.27

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

@aarontrowbridge

Copy link
Copy Markdown
Member Author

Review notes (for @jeonghun-jj-lee)

The behavioral core is small: DirectTrajOptProblem construction now applies a default $\Delta t$ window [nominal/4, nominal×4] when the timestep carries no bounds (previously only a zero floor), and the injected-bounds warning states the three free-time modes instead of recommending timesteps_all_equal "to fix" timesteps (it doesn't — that's uniform mesh with free duration by design; see Piccolo.jl#317).

Three things that make review faster:

  1. Problems with explicit $\Delta t$ bounds are untouched — "explicit wins" is tested. All benchmark + convergence problems bound $\Delta t$ explicitly, so the changed path is unreachable on the CI suites.
  2. The first benchmark report on this PR shows scary deltas that are not real. The published series entry it diffed against was from Aug 21 — six days of main (fix(integrators): restore the multi-state BilinearIntegrator constructor #139/release: v0.10.1 — multi-state BilinearIntegrator restore #140, registry drift) plus shared-runner wall noise (rows with byte-identical allocations and identical iterations moved ±10–30% in wall). A fresh main-branch baseline has been dispatched to settle it same-day; ci: weekly scheduled benchmark baseline on main #147 keeps the anchor fresh weekly going forward.
  3. Evidence base for the semantics: drift-dominated 1-qubit benchmark — unbounded free time ran the padded duration sum to $10^5\times$ nominal with fidelity collapse; pinned-but-variable timesteps stall at $F=0.65$ where a derived-timestep prototype solves to $10^{-12}$ (same problem, same seed). Full survey in Piccolo.jl#317.

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