Skip to content

Move trunc out of bond truncation algorithm#362

Draft
Yue-Zhengyuan wants to merge 1 commit intoQuantumKitHub:masterfrom
Yue-Zhengyuan:als-3site
Draft

Move trunc out of bond truncation algorithm#362
Yue-Zhengyuan wants to merge 1 commit intoQuantumKitHub:masterfrom
Yue-Zhengyuan:als-3site

Conversation

@Yue-Zhengyuan
Copy link
Copy Markdown
Member

@Yue-Zhengyuan Yue-Zhengyuan commented Apr 18, 2026

This PR moves trunc::TruncationStrategy out of bond truncation algorithms ALSTruncation and FullEnvTruncation.

The motivation is to make supporting FixedSpaceTruncation and SiteDependentTruncation in NTU (#144) easier, so I don't need to reconstruct the bond truncation algorithm struct every time I switch the bond to be truncated (since each bond have a different trunc).

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/algorithms/truncation/bond_truncation.jl 94.52% <100.00%> (ø)
src/algorithms/truncation/fullenv_truncation.jl 96.77% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Yue-Zhengyuan Yue-Zhengyuan requested a review from leburgel April 18, 2026 08:24
Copy link
Copy Markdown
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

This does look in general quite different from how we structure the rest of our function signatures, which typically have the truncation strategy in the algorithm... I'm not sure if I really understand the benefit, isn't this just a matter of using @set alg.trunc = ...?

@Yue-Zhengyuan
Copy link
Copy Markdown
Member Author

Currently the NTU struct looks like

@kwdef struct NeighbourUpdate <: TimeEvolution
    "Bond truncation algorithm after applying time evolution gate"
    opt_alg::Union{ALSTruncation, FullEnvTruncation} =
        ALSTruncation(; trunc = truncerror(; atol = 1.0e-10))
    ...
end

I feel it a bit weird to set trunc = FixedSpaceTruncation() in opt_alg, since the bond_truncate function that ultimately uses opt_alg does not know the original bond space before applying the gate, and FixedSpaceTruncation does not make sense in this narrow context. So I want to make trunc directly a field in NeighbourUpdate.

Though I admit that this is still not ideal, and I'm surely open to suggestions.

@Yue-Zhengyuan Yue-Zhengyuan marked this pull request as draft April 19, 2026 07:48
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