aci: stop the elementwise sweep once the rank saturates at max_bond_dim - #591
Merged
Conversation
The elementwise ACI convergence loop only exits on the tolerance branch of convergence_criterion_like_julia, which requires the pivot error to reach options.tolerance before the rank-plateau check can apply. Once the solution rank reaches options.max_bond_dim the sweep can no longer add pivots, so under a binding cap the tolerance can never be met and the loop burns every remaining max_iters sweep at full rank. Add the missing exit: break when the rank has stayed at max_bond_dim for min_iters consecutive sweeps. This is the all(lastranks .>= maxbonddim) disjunct of the Julia convergencecriterion that the rest of the criterion already ports, over the same trailing window. tensor4all-treetci gained the equivalent exit in #575. The min_iters dwell means the loop does not stop on the sweep where the cap is first reached, when pivot re-selection inside the fixed rank can still improve the error, and min_iters == 0 disables the exit exactly as it disables the tolerance criterion. The default max_bond_dim of usize::MAX is unreachable, so unconstrained runs are unaffected. Found while running tensor4all/tensor4all-benchmark case elementwise_gauss2d_scaling. At N=32 (chi_in=116, tolerance 1e-8, max_bond_dim=chi_in) ACI reached rank 116 on sweep 2 and then ran all 20 sweeps with the error flat near 2.1e-8. Measured aci wall time for BENCH_NS=8,16,32,64 BENCH_RUNS=1: N before after 8 0.0434s 0.0440s 16 0.0781s 0.0804s 32 0.8066s 0.0772s 64 1.7250s 0.1522s Sampled relative errors stay at the same order (N=32: 1.04e-8 to 4.95e-9, N=64: 2.04e-8 to 1.85e-8), and the zipup_treetn and fit_treetn arms are unchanged. Because a capped run now returns with the last errors entry above tolerance, document that outcome on both elementwise_batched and AciResult::errors so callers read it as rank limiting rather than as a failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Repository rules reviewRepository rules review (a6a51de4cc9a082265eba773bbaf65b828a43512...da55383b74a5ff71df7c7d764b1878534afd3a67) Verdict: pass Findings: - [warn] llm-skipped (External LLM Review) <unknown>: External LLM review was skipped External LLM review is permanently disabled in this repository; deterministic rule checks run instead. |
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
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.
Problem
elementwise_batchedincrates/tensor4all-aci/src/elementwise.rsonly exits its sweep loop through the tolerance branch ofconvergence_criterion_like_julia, which requireserrors.last() <= options.tolerancebefore the rank-plateau check can apply. Once the solution rank reachesoptions.max_bond_dimthe sweep has no remaining freedom to add pivots, so under a binding cap the tolerance can never be met. The loop then burns every remainingmax_iterssweep at full rank doing nearly dead work.Found while running tensor4all/tensor4all-benchmark case
elementwise_gauss2d_scaling. At N=32 (chi_in=116, tolerance1e-8,max_bond_dim=chi_in) ACI reaches rank 116 on sweep 2 and then runs all 20 sweeps, per-sweep ranks[112, 116, 116, ..., 116], with the pivot error metric flat near2.1e-8. Uncapped, the same problem converges in 3 sweeps. Same story at N=64.Fix
Break when the solution rank has stayed at
options.max_bond_dimforoptions.min_itersconsecutive sweeps, added as a separaterank_is_saturatedcheck next to the existing criterion rather than folded into it, so the ported criterion keeps its shape.Design notes:
all(lastranks .>= maxbonddim)disjunct of the Juliaconvergencecriterionthatconvergence_criterion_like_juliaotherwise ports, evaluated over the same trailing window.tensor4all-treetcigained the equivalent exit in its own sweep loop in treetci: add missing convergence check to optimize's sweep loop #575, so the two sweep loops now agree.min_iterssweeps at the cap is deliberate. Breaking on the very sweep the cap is first hit would cut off pivot re-selection inside the now fixed rank, which can still lower the error; requiring the rank to sit at the cap for a full window keeps that opportunity.min_iters == 0disables the exit, matching how it disables the tolerance criterion.max_bond_dimdefaults tousize::MAX, which is unreachable, so unconstrained runs take exactly the path they took before. Behavior for non-saturated runs is unchanged.A capped run now returns normally with the last
AciResult::errorsentry abovetolerance. Since that could be misread as a failure, both theelementwise_batchedrustdoc and theAciResult::errorsfield doc now state that a bindingmax_bond_dimfinishes above tolerance by design, and point callers at the lastranksentry to tell a rank-limited run from a converged one.Verification
End-to-end, benchmark
elementwise_gauss2d_scalingwithBENCH_NS=8,16,32,64 BENCH_RUNS=1 BENCH_WARMUPS=0, aci arm wall time:N=8 and N=16 do not hit the cap and are unchanged within run to run noise. Sampled relative errors stay at the same order (N=32:
1.04e-8to4.95e-9; N=64:2.04e-8to1.85e-8), and thezipup_treetnandfit_treetnarms are unaffected. Thebeforecolumn was measured at the benchmark's current pin7cfec22, which is identical tomainforcrates/tensor4all-aci(git log 7cfec22..main -- crates/tensor4all-aciis empty).Tests:
capped_elementwise_run_stops_once_rank_saturates: two value-dependent rank-4 trains whose pointwise product genuinely needs rank 16, run againstmax_bond_dim: 4with an unreachable1e-10tolerance. It asserts the cap is actually binding, that the run uses at mostmin_iters + 2sweeps, and that the trailingranksentries equal the cap. On the unfixed loop it fails withexpected an early exit, got 20 sweeps with ranks [4, 4, ..., 4].rank_saturation_needs_a_full_dwell_at_the_capcovers the dwell boundary, the unreachable default cap, andmin_iters == 0.cargo nextest run --release --workspace --exclude library-panic-audit: 2718 passed. The excludedlibrary-panic-audittool tests fail on this machine independently of this branch, which touches nothing undertools/.cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warnings,cargo doc -p tensor4all-aci --no-deps, andpython3 scripts/repository-rules-review.py --base main --worktree --dry-run(verdict: pass) all clean.🤖 Generated with Claude Code