Skip to content

Add PEPS docs, examples, and benchmarks#46

Open
bernalde wants to merge 9 commits into
feature/issue-39-jump-peps-attrsfrom
docs/issue-40-peps-examples-benchmarks
Open

Add PEPS docs, examples, and benchmarks#46
bernalde wants to merge 9 commits into
feature/issue-39-jump-peps-attrsfrom
docs/issue-40-peps-examples-benchmarks

Conversation

@bernalde

@bernalde bernalde commented May 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds a user-facing PEPS backend guide covering backend selection, optional installation, supported topologies, direct API usage, JuMP attributes, metadata, parameters, limitations, and local benchmarks.
  • Adds deterministic square and king benchmark scripts under benchmarks/ plus a shared helper module and benchmark README.
  • Adds a lightweight benchmark-helper smoke test to normal tests without running the benchmark scripts or requiring optional SpinGlassPEPS packages.
  • Updates docs navigation and the SpinGlassPEPS architecture page to point users to the new PEPS backend guide.
  • Clarifies that direct PEPSBackend(contraction=...) and JuMP "peps_strategy" configure the same setting, and documents the current SVD alias behavior.

Tests run

  • git diff --check - passed.
  • julia +1.12 --project=. -e 'using Pkg; Pkg.instantiate()' - passed.
  • julia +1.12 --project=docs/ -e 'using Pkg; Pkg.instantiate()' - passed.
  • julia +1.12 --project=. -e 'using Test, TenSolver; include("test/benchmarks.jl")' - passed.
  • julia +1.12 --project=docs/ docs/make.jl - passed.
  • julia +1.12 --project=. benchmarks/peps_square.jl - passed. PEPS row skipped because optional SpinGlass component packages are not installed.
  • julia +1.12 --project=. benchmarks/peps_king.jl - passed. PEPS row skipped because optional SpinGlass component packages are not installed.
  • julia +1.12 --project=. -e 'using Pkg; Pkg.test(; coverage=false)' - passed. The optional SpinGlassPEPS extension solve remains reported as skipped/broken when optional component packages are not installed.
  • GitHub Actions for the current head passed the configured Documentation workflow, documenter/deploy, and Julia lts/1/pre test matrix.

Notes

  • The local command list above records checks run with julia +1.12; CI remains the source of truth for the repository's configured Julia matrix.
  • The benchmark scripts are intentionally outside normal CI. Normal CI only loads the helper and checks tiny deterministic problem construction/brute force behavior.
  • The PEPS docs examples are intentionally plain julia blocks rather than doctests because they require optional SpinGlass component packages that are not expected to resolve in the ordinary docs CI environment.
  • local_dimension is documented in the parameter guide but intentionally left out of the minimal examples; the examples keep to the core square/king CPU path.
  • Pegasus and Zephyr benchmark scripts are not included because those topologies are not exposed by the current TenSolver PEPS backend.
  • The current pushed head is restacked on the updated Add JuMP PEPS backend attributes #45 branch with append-only merge commits.

Closes #40

@bernalde
bernalde force-pushed the feature/issue-39-jump-peps-attrs branch from 078e097 to 873f02b Compare May 17, 2026 01:26
@bernalde
bernalde force-pushed the docs/issue-40-peps-examples-benchmarks branch from 9f20a3f to 6bda248 Compare May 17, 2026 01:27
@bernalde
bernalde force-pushed the feature/issue-39-jump-peps-attrs branch from 873f02b to ab04073 Compare May 19, 2026 23:44
@bernalde
bernalde force-pushed the docs/issue-40-peps-examples-benchmarks branch 2 times, most recently from 3abd45d to b40dc57 Compare May 19, 2026 23:52
@bernalde
bernalde force-pushed the docs/issue-40-peps-examples-benchmarks branch from 72ffabb to 93c2551 Compare May 20, 2026 04:13

@bernalde bernalde left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub rejected REQUEST_CHANGES because the authenticated account is the PR author, so this is submitted as COMMENT; the review disposition should be treated as request changes.

  1. Blocking issues
  • The PEPS user guide documents an install path that does not currently resolve with the registered component packages in a clean Julia 1.11.5 TenSolver environment.
  • The PEPS examples do not load the weak-dependency trigger packages needed to activate TenSolverSpinGlassPEPSExt, so the shown minimize(...; backend) calls would still hit the core unavailable-backend path once the resolver issue is fixed.
  1. Nonblocking issues
  • None.
  1. Questions
  • None.
  1. Tests run and outcomes
  • git diff --check origin/feature/issue-39-jump-peps-attrs...HEAD passed.
  • julia --project=. -e 'using Test, TenSolver; include("test/benchmarks.jl")' passed: 8/8 benchmark helper tests.
  • julia --project=. benchmarks/peps_square.jl passed; DMRG and brute force succeeded, PEPS was skipped because the optional SpinGlass packages are not installed.
  • julia --project=. benchmarks/peps_king.jl passed; DMRG and brute force succeeded, PEPS was skipped because the optional SpinGlass packages are not installed.
  • julia --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()' passed.
  • julia --project=docs/ docs/make.jl local passed.
  • julia --project=. -e 'using Pkg; Pkg.test()' passed; optional SpinGlassPEPS extension tests were skipped because the component packages are not available.
  • Clean optional-stack check with julia --project=/tmp/tensolver-pr46-opt -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.add(["SpinGlassNetworks", "SpinGlassEngine", "SpinGlassTensors"])' failed with an unsatisfiable Parsers conflict through SpinGlassNetworks/CSV and TenSolver's QUBOTools/InlineStrings environment.
  1. Merge decision
    I would not merge this until the blocking issues above are addressed.

Comment thread docs/src/peps_backend.md Outdated
Comment thread docs/src/peps_backend.md
@bernalde

Copy link
Copy Markdown
Member Author

Pushed commit:

Main changes:

  • Replaced the PEPS guide's actionable Pkg.add([...]) install path with the current state: the registered SpinGlass component stack does not resolve with TenSolver's QUBOTools/InlineStrings environment, so the PEPS bridge is scaffolding until upstream compat is fixed.
  • Added import SpinGlassNetworks, import SpinGlassEngine, and import SpinGlassTensors to the direct and JuMP PEPS examples so future PEPS-capable sessions activate TenSolverSpinGlassPEPSExt before solving.
  • Updated related examples, architecture notes, the docs index, benchmark README, and benchmark skip text to say PEPS requires available and importable component packages and should skip in ordinary registered-package environments today.

Tests run:

  • git diff --check passed.
  • julia +1.12 --project=. -e 'using Test, TenSolver; include("test/benchmarks.jl")' passed: Benchmark helpers: 8 pass.
  • julia +1.12 --project=. benchmarks/peps_square.jl passed: brute force and DMRG ran, PEPS skipped because the SpinGlassPEPS component stack is unavailable or not importable.
  • julia +1.12 --project=. benchmarks/peps_king.jl passed: brute force and DMRG ran, PEPS skipped for the same reason.
  • julia +1.12 --project=docs/ docs/make.jl local initially failed because the docs manifest referenced a stale MbedTLS_jll source path; after julia +1.12 --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()', rerunning julia +1.12 --project=docs/ docs/make.jl local passed. The temporary absolute path written to docs/Project.toml by Pkg.develop was restored before committing.
  • julia +1.12 --project=. -e 'using Pkg; Pkg.test()' passed. Optional SpinGlassPEPS extension tests remain expected broken/skipped when the component packages are not available.

Comments intentionally not addressed:

  • None.

Remaining risks/follow-up:

  • The real optional SpinGlassPEPS solve path is still blocked by the registered component resolver conflict. This PR now documents that limitation instead of presenting the install path as actionable.

@bernalde bernalde left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub rejected APPROVE because the authenticated account is the PR author, so this is submitted as COMMENT; the review disposition should be treated as approve.

  1. Blocking issues
  • None.
  1. Nonblocking issues
  • None.
  1. Questions
  • None.
  1. Tests run and outcomes
  • git diff --check origin/feature/issue-39-jump-peps-attrs...HEAD passed.
  • julia --version: Julia 1.11.5.
  • julia --project=. -e 'using Pkg; Pkg.instantiate()' passed.
  • julia --project=. -e 'using Test, TenSolver; include("test/benchmarks.jl")' passed: 8/8 benchmark helper tests.
  • julia --project=. benchmarks/peps_square.jl passed: brute force and DMRG ran; PEPS skipped with the updated unavailable/importable component-stack message.
  • julia --project=. benchmarks/peps_king.jl passed: brute force and DMRG ran; PEPS skipped with the updated unavailable/importable component-stack message.
  • julia --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()' passed.
  • julia --project=docs/ docs/make.jl local passed.
  • julia --project=. -e 'using Pkg; Pkg.test()' passed. Optional SpinGlassPEPS extension tests remain expected broken/skipped because the component packages are not available.
  • Clean optional-stack check with julia --project=/tmp/tensolver-pr46-opt -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.add(["SpinGlassNetworks", "SpinGlassEngine", "SpinGlassTensors"])' failed with the documented Parsers resolver conflict. This matches the updated PEPS scaffolding documentation.
  • GitHub Actions are green for documentation and the Julia lts/1/pre matrix on Ubuntu, Windows, and macOS aarch64.
  1. Merge decision
    The current PR is merge-ready as PEPS scaffolding/documentation/benchmark support, with the real optional-stack solve still explicitly gated on upstream resolver-compatible SpinGlass component releases. It should be merged as-is.

@bernalde bernalde left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Senior maintainer review. Note: I am the PR author, so GitHub only permits a COMMENT event from me; a formal approval or change-request must come from another maintainer. No blocking issues found.

Diff verification: confirmed against the stacked base feature/issue-39-jump-peps-attrs (not main; top of the 6-PR stack). merge-base f674a37; git diff <merge-base>..HEAD --stat = 13 files, +723 / -15, matching the PR's reported totals. No src/ or ext/ changes (git diff <mb>..HEAD -- src ext is empty) — a pure docs/benchmarks/tests PR; no code sneaking in.

Issue intent (#40): docs/examples/benchmarks setting correct expectations. Satisfied: backend overview (DMRG default / PEPS optional-experimental), installation with Julia-version and optional-GPU notes, supported-topologies table honestly marking square/king Implemented and Pegasus/Zephyr Planned (matching the code, which only exposes SquareGrid/KingGrid), usage examples (direct API, JuMP backend=:peps, metadata), parameter guide, and an explicit limitations section stating DMRG remains default and PEPS does not supersede TenSolver. No over-claiming of PEPS superiority.

Verified every API surface used in docs/benchmarks against the stack's real code: PEPSBackend kwargs (incl. no_cache), SquareGrid/KingGrid, PEPSSolution fields, sample(::PEPSSolution), all JuMP peps_* attributes, metadata keys, and the :identity/:all/:svd symbols — all match. No registered doctest depends on the heavy PEPS stack (the PEPS JuMP example is a plain ```julia block, correctly excluded from executed doctests), so the CI doc build does not require SpinGlassPEPS.

Nonblocking

  1. Docs use contraction = :svd for the direct PEPSBackend constructor (docs/src/peps_backend.md:81, :137) but the JuMP attribute is peps_strategy (:181). Both are correct and internally consistent; add one sentence noting they are the same setting to avoid user confusion.
  2. The PR body reports tests run under julia +1.12, but CI/this environment is Julia 1.10.11. I re-ran on 1.10.11 and everything passes — note the claimed commands used a different Julia than CI.

Question
3. The PEPS examples in examples.md/peps_backend.md are intentionally non-executed (plain julia blocks, no jldoctest/@example) because they import SpinGlass packages. Confirmed correct; raising only so the reviewer agrees these should stay non-executed rather than become doctests.

Tests/build run (Julia 1.10.11):

  • julia --project=docs -e 'Pkg.develop(path=pwd()); Pkg.instantiate()' — success.
  • julia --project=docs docs/make.jl local — success: doctests pass, CrossReferences + CheckDocument pass with no warnings (all @ref resolve, new page registered in nav).
  • julia --project=. -e 'using Pkg; Pkg.test()' — passed, incl. new Benchmark helpers 8/8 and Doctests 1/1; core suite unaffected.
  • Parse-checked the three benchmark scripts; ran benchmarks/peps_square.jl to completion — brute force and DMRG both reach the optimum (-2.95075, gap 0), PEPS row skipped cleanly with "SpinGlassPEPS component stack unavailable," exactly as documented.

Merge readiness: Ready to merge (pending normal human review). Docs are accurate against the real API, the docs build and full test suite pass on CI Julia (1.10.11), no registered doctest depends on the heavy stack, benchmarks are runnable and honest, the new page is registered with resolving cross-references, and expectations are set correctly per #40 without over-claiming. Only optional nonblocking polish (clarify contraction/peps_strategy equivalence). Top of a stack based on feature/issue-39-jump-peps-attrs; merge after its base lands. The formal verdict must come from another maintainer.

Comment thread docs/src/peps_backend.md

@bernalde bernalde left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Senior-maintainer review of PR #46 "Add PEPS docs, examples, and benchmarks". Reviewed the head (01e0e66) diffed against the merge-base with the #45 base branch feature/issue-39-jump-peps-attrs (f674a37), reproducing the reported totals exactly: +723/-15 across 13 files. I only evaluated changes introduced by #46; the PEPS backend/topology/extension code itself is inherited from the #41-#45 stack and is out of scope here.

Disclosure on verification: the review sandbox denied julia/git for any command that changes directory or instantiates the docs env (as expected), so I could not run the doc build or doctests locally. I verified every doc claim by reading the head sources via the GitHub API (src/TenSolver.jl, src/solver.jl, src/solution.jl, ext/TenSolverSpinGlassPEPSExt.jl) and confirmed CI on the exact head SHA: the build job (which runs doctests) and documenter/deploy both succeeded, so the doc build is healthy and @ref cross-references resolve. Note CI ran against the stale base chain, not current main.

Accuracy check (docs vs the code the stack actually introduces) - all verified correct:

  • TenSolver.PEPSBackend, TenSolver.SquareGrid(m, n[, spins_per_site]), TenSolver.KingGrid(...) signatures and the beta/bond_dim/max_states/cutoff_prob/contraction/transformations/local_dimension/onGPU/num_sweeps/no_cache keywords match src/solver.jl.
  • The JuMP attribute names in both examples.md and peps_backend.md (backend, peps_layout, peps_topology, peps_beta, peps_bond_dim, peps_max_states, peps_cutoff_prob, peps_strategy, peps_transformations) match the QUBODrivers.@setup attribute block.
  • PEPSSolution field references (solution.states, .energies, .probabilities, .metadata) and sample(solution) match src/solution.jl.
  • Metadata keys documented (topology, selected_transformation, largest_discarded_probability, and the QUBODrivers "peps" key with candidate_states/effective_time) match _metadata/_add_backend_metadata! in the extension and src/TenSolver.jl.
  • The "construction allowed, solve errors when packages absent" claim matches _backend_error/_solve_ising dispatch.
  • All PEPS code blocks in the docs are plain julia (not jldoctest), so no doctest attempts the optional SpinGlassPEPS stack; the gated benchmark helpers (has_peps_components/load_peps_components) correctly skip the PEPS row when the component packages are absent. This matches the issue's requirement that DMRG docs run without the optional dependency.

Issue closure: Closes #40 is appropriate. Issue #40 ("6/6") is explicitly scoped to docs + examples + benchmark scripts, and the PR delivers each acceptance-criterion item (backend overview, installation/optional-extension notes, supported topologies, direct-API + JuMP/QUBODrivers + metadata examples, parameter guide, limitations, non-CI benchmark scripts, and lightweight smoke tests). No partial-gap that would warrant downgrading to Refs #40.

Nonblocking and questions below. No blocking issues are introduced by #46 itself; the only thing preventing merge is structural (stacked base + staleness), detailed in the summary.

Summary

  1. Blocking: none introduced by #46.
  2. Nonblocking: peps_backend.md lists :auto, :svd, :svd_truncate, :zipper as if four distinct contraction strategies, but _strategy in ext/TenSolverSpinGlassPEPSExt.jl maps :auto/:svd/:svd_truncate all to SVDTruncate (only :zipper -> Zipper); consider noting these three are currently equivalent.
  3. Questions: (a) Issue task 2 asks to state Julia compatibility "if the PEPS bridge requires Julia 1.11"; the docs only say it "historically required newer Julia versions" - is there a concrete minimum worth pinning? (b) local_dimension/peps_truncation is documented in the parameter guide but not shown in any example; intentional?
  4. Tests/doc-build: could not run julia/git locally (sandbox denied dir-changing/instantiate commands; tried once with sandbox disabled). Relied on CI at the exact head SHA 01e0e66: all 9 Julia matrix jobs + build (doctests) + documenter/deploy succeeded. Diff totals and merge-base verified manually.
  5. Merge-readiness: NOT mergeable yet, for structural reasons, not content. This PR's base is feature/issue-39-jump-peps-attrs (head of #45), so it cannot merge until the entire base chain #41-#45 merges. The stack was cut from old main (ca27880); current origin/main (v0.2.0, 9a3dd5c, with #49/#53/#54) has diverged heavily in src/TenSolver.jl (~143 lines), src/solver.jl, and test/Project.toml - the same files the stack rewrites - so expect non-trivial rebase conflicts (including test/Project.toml, which both sides modify). CI passed only against the stale base. Recommendation: rebase the base chain onto current main, re-run CI, then merge bottom-up.

Formal verdict: GitHub blocks an Approve/Request-changes from me as PR author, so this is posted as a COMMENT. Content-wise I would approve #46 on its own merits once the base chain lands and is rebased onto current main; another maintainer should record the formal approval.

Comment thread docs/src/peps_backend.md Outdated
@bernalde

Copy link
Copy Markdown
Member Author

Pushed commits:

Main changes:

  • Merged the updated feature/issue-39-jump-peps-attrs base into this PR branch without rewriting history.
  • Added a PEPS guide note that JuMP "peps_strategy" maps to direct PEPSBackend(contraction=...).
  • Clarified that :auto, :svd, and :svd_truncate currently use SVD truncation, while :zipper selects zipper contraction.
  • Updated the guide to document QUBODrivers PEPS metadata under metadata["tensolver"]["peps"].

Tests run:

  • git diff --check passed.
  • julia +1.12 --project=. -e 'using Pkg; Pkg.instantiate()' passed.
  • julia +1.12 --project=docs/ -e 'using Pkg; Pkg.instantiate()' passed.
  • julia +1.12 --project=. -e 'using Test, TenSolver; include("test/benchmarks.jl")' passed. Benchmark helpers: 8 pass.
  • julia +1.12 --project=docs/ docs/make.jl passed.
  • julia +1.12 --project=. benchmarks/peps_square.jl passed. Brute force and DMRG ran; PEPS skipped because the optional SpinGlassPEPS component stack is unavailable or not importable.
  • julia +1.12 --project=. benchmarks/peps_king.jl passed. Brute force and DMRG ran; PEPS skipped for the same reason.
  • julia +1.12 --project=. -e 'using Pkg; Pkg.test(; coverage=false)' passed.

Comments intentionally not addressed:

  • None.

Remaining risks/follow-up:

  • The real optional SpinGlassPEPS solve path is still blocked by the registered component resolver conflict.
  • GitHub Actions checks for this pushed head are pending.

@bernalde

bernalde commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

Addressed the current review-comment pass for PR #46.

Changes made:

  • No repository commits were needed; the branch was already up to date.
  • Updated the PR body metadata to clarify that local verification used julia +1.12, while GitHub Actions is the source of truth for the configured Julia matrix.
  • Clarified that the PEPS documentation snippets are intentionally plain julia examples rather than doctests because optional SpinGlass dependencies are not expected in ordinary docs CI.
  • Clarified that local_dimension is documented in the parameter guide but intentionally omitted from the minimal examples.

Review threads:

  • Confirmed there are no unresolved inline review threads to reply to.

Verification:

  • No local tests were run in this pass because no repository files changed.
  • Rechecked the live PR state for head 25efd4557baf367d94d73f90d92b14e81d92b525; GitHub checks are green for Documentation, documenter/deploy, and the configured Julia test matrix.
  • A no-op push confirmed docs/issue-40-peps-examples-benchmarks is already up to date.

Remaining notes:

  • The PR is still stacked on feature/issue-39-jump-peps-attrs; merge sequencing should follow that base stack.

@bernalde

Copy link
Copy Markdown
Member Author

Heads-up for this PR's post-#44/#45 rebase: the #101 docs review (now merged) established documentation principles from @iagoleal that this PR's peps_backend.md + benchmark docs should follow when reworked:

  • No internals in public documentation (#101 inline): "In general, do not talk about internals in the public documentation. The public documentation should focus on the API and other relevant information to the users." The current guide documents scaffolding-level details (internal types, dependency-conflict mechanics) that should either move to docs/internal/ or be trimmed to user-facing behavior.
  • Dedicated-page pattern: novel features get their own nav page with theory/design principles, a mathematical-formulation → constructor mapping, restrictions, and cost expectations (the structure requested for constraints applies just as well to the PEPS backend page: topology mapping, supported couplings, contraction cost knobs).
  • README stays link-only for experimental surfaces (#101 inline): a short snippet plus a direct link to the docs page, with an explicit experimental note — no type lists or mechanics in the README.
  • No stochastic doctests (#28 inline): "It is better to not have random doctests" — doctests must pin outputs on unique-optimum instances; anything seed-dependent belongs in plain julia blocks.

Cross-referenced from a sweep of review comments on closed PRs, 2026-07-16.

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.

2 participants