Code review fixes: correctness, robustness, packaging, ITensorMPS 0.4#2
Merged
Conversation
Correctness / robustness: - MPO match_energy!: _correction_time returned a purely imaginary tdvp time argument, so exp(t*op) was unitary and conserved the energy. Return real -alpha*error so it performs imaginary-time evolution and converges, matching the dense correction path. - Dense match_energy!: stop rolling back on slow-but-positive progress (drop the -tol slack) and add a convergence check; apply the correction once per unique bond so the effective step no longer scales with the schedule length. - DMT _mat_trunc!: replace the exact ==0 connector guard with a relative tolerance; skip only the connector subtraction (never the truncation) so maxdim is enforced for traceless operators and a near-singular M[1,1] no longer blows the connector up. - trajectory_refine!: NaN selector scores no longer silently keep the initial state; add NaN-aware comparison and a warning. - TDVPEvolution: reject solver_kwargs keys that would silently override the dedicated arguments (time_step/nsteps/reverse_step/updater_backend/updater/ normalize). Packaging: - Drop unused EDKit and Plots dependencies. Regenerating the Manifest removes the Qt stack pulled in by Plots (247 -> 109 packages), which had broken Pkg.test(). Kept dependencies remain within their existing compat bounds. Docs: - Document the pauli_components (sqrt2)^L normalization mismatch with the operator-space state basis; fix the incorrect pauli_matrices().Z doctest output; note the Chebyshev rescaling safety margin and band-edge behavior; reorder the chebyshev includes (types -> reconstruction -> moments); drop the redundant sqrt/abs2 round-trip in bond_entropy. Tests: - Add test/test_oracles.jl with exact-diagonalization oracles (TEBD and TDVP vs exp(-iHt), the periodic kron(op_N, op_1) convention, and the KPM sum rule), plus regression tests for each fix above. Full Pkg.test() passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…idation, docs) Follow-up to the previous review-fix commit, resolving the items that were deferred there as design decisions: - MPO match_energy!: replace the full -correction_time rollback with a proportional one (mix rollback on sign-flip overshoot, full undo only on stagnation), mirroring the dense path, so an overshooting step lands near the target instead of bouncing back to the pre-step energy. - DMTOptions: add a dmt_step!(psi, gate, bond, opts::DMTOptions) overload so the validated/exported options struct is actually consumed instead of being dead. - DMT validation: _validate_dmt_step now checks the Pauli site dimension for every span, including span == 1, so a single-site gate on a non-Pauli site is rejected rather than silently accepted. - Docs: clarify that energy_cutoff! filters the local effective problem (not the global spectrum) and that its error estimate is a heuristic monitor rather than an RMS; pin the load-bearing basis[1] = normalize(tensor) invariant in _project_energy_window; note the gate_maxdim / maxdim / connector_buffer accuracy interaction; document the refinement tie-break and NaN handling. Adds regression tests for the proportional rollback, the DMTOptions overload, and single-site Pauli validation. Full test suite (julia --project=. test/runtests.jl) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Improvements to the items left open after the review-fix commits: - Chebyshev rescaling helpers: add chebyshev_rescaling(emin, emax; margin) and rescale_hamiltonian(H, emin, emax; margin, cutoff). They map a Hamiltonian's spectrum strictly inside the Chebyshev band (extreme eigenvalues at ±(1 - margin)) and return the ChebyshevRescaling for reconstruction, closing the band-edge foot-gun where weight mapped to ±1 was silently dropped. Both are exported and tested. - ITensorMPS 0.4 support: widen compat to "0.3.44, 0.4". 0.4.1 resolves with ITensors 0.9.30 (no ITensors cascade); the full Pkg.test() suite passes on 0.4.1, and the Manifest now locks 0.4.1. - Doctests: convert the two docstring examples (pauli_matrices, pauli_basis_state) to jldoctest blocks with stable value-checking output, add DocMeta.setdocmeta! in docs/make.jl so the package is in scope, and add value-based example tests to the suite. `doctest(MPSToolkit)` passes. - entanglement_spectrum: drop the redundant copy before the non-mutating orthogonalize (it already preserves the input), halving the per-score allocation during refinement; add a no-mutation regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jayren3996
added a commit
that referenced
this pull request
Jun 1, 2026
The docs build runs `--project=docs`, but `docs/Project.toml` still listed EDKit and Plots after they were removed from the main project as unused. The committed `docs/Manifest.toml` pinned `Qt6Base_jll = 6.10.2+1` (pulled in only by Plots), which the General registry has since superseded with `6.10.2+2`, so `Pkg.develop` + `Pkg.instantiate` could no longer resolve and the Documentation workflow failed (~28s) on PR #2 and the main merge. Neither Plots nor EDKit is referenced anywhere in docs/; remove both from `docs/Project.toml` and regenerate a clean `docs/Manifest.toml` (247 -> 130 packages, no Qt6/GR/Plots tree). Verified locally: `julia --project=docs docs/make.jl` runs doctests and renders all pages with no errors. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Multi-round code review of the toolkit, with every finding verified empirically (running Julia / exact-diagonalization oracles) before fixing, and every fix written test-first.
Confirmed bugs fixed
match_energy!was a no-op on energy (critical):_correction_timepassed a purely imaginary time totdvp, soexp(t·op)was unitary and conserved ⟨H⟩. Now returns a real interval (imaginary-time cooling), matching the dense path, and uses a proportional rollback on overshoot.match_energy!silently stalled: rolled back on slow-but-positive progress; now only rolls back on non-improvement and has a convergence check.alphais schedule-independent._mat_trunc!skipped truncation for traceless operators and blew up on a near-singular connector: exact==0guard replaced with a relative tolerance that still enforcesmaxdim.solver_kwargssilently overrode dedicated fields: reserved keys now rejected at construction.Packaging / compat
EDKitandPlotsdeps (Plots' Qt stack had brokenPkg.test()); Manifest 247 → 109 packages.Pkg.test()).Docs / tests / cleanup
chebyshev_rescaling/rescale_hamiltonianhelpers to close the band-edge foot-gun.jldoctestblocks +DocMeta.setdocmeta!); fixed the incorrectpauli_matrices().Zoutput; documented thepauli_components(√2)^L trap, the DMTgate_maxdiminteraction, and theenergy_cutoff!local-vs-global limitation.exp(-iHt), periodic gate ordering, KPM sum rule) plus regression tests for every fix.entanglement_spectrum(perf).Full
Pkg.test()passes on the resolved versions.🤖 Generated with Claude Code