Skip to content

Refactor operator types#347

Draft
lkdvos wants to merge 19 commits intomasterfrom
ld-operatortypes
Draft

Refactor operator types#347
lkdvos wants to merge 19 commits intomasterfrom
ld-operatortypes

Conversation

@lkdvos
Copy link
Copy Markdown
Member

@lkdvos lkdvos commented Mar 31, 2026

This is a WIP implementation of updated structs for our operators, with the goal of being less painful for the compiler, while being a bit more natural for representing sums as dictionaries (commuting terms -> order does not matter), while products are vectors (non-commuting factors -> order retained).

Unfortunately I will not find the time to finish this, so if anyone feels like continuing the work, go ahead!
I do think most of what I have in mind is here, the general ideas are in place and this is mostly about cleaning up and dealing with Zygote.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 84.57711% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/operators/localoperator.jl 77.94% 15 Missing ⚠️
src/algorithms/time_evolution/trotter_gate.jl 83.90% 14 Missing ⚠️
src/algorithms/contractions/bp_contractions.jl 92.00% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/algorithms/contractions/localoperator.jl 96.85% <100.00%> (-0.02%) ⬇️
src/algorithms/time_evolution/gaugefix_su.jl 89.65% <100.00%> (ø)
src/algorithms/time_evolution/simpleupdate.jl 95.20% <100.00%> (+2.45%) ⬆️
src/algorithms/toolbox.jl 97.69% <100.00%> (ø)
src/operators/models.jl 80.48% <100.00%> (ø)
src/algorithms/contractions/bp_contractions.jl 86.66% <92.00%> (+0.52%) ⬆️
src/algorithms/time_evolution/trotter_gate.jl 87.93% <83.90%> (-4.11%) ⬇️
src/operators/localoperator.jl 82.53% <77.94%> (-6.18%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leburgel
Copy link
Copy Markdown
Member

leburgel commented Apr 1, 2026

I'll try to have a decent look soon, but some comments/notes already:

  • It seems the MatrixAlgebraKit.jl v0.6.5 release broke our QR CTMRG optimization when using a Float64 scalar type. This is now failing in the Heisenberg example, also on master. No clue what changed and this definitely needs to be sorted out, but technically unrelated.
    • UPDATE: this turned out not to be a fundamental issue, it just seems that some combination of changes here and in MAK v0.6.5 broke things for the specific seed we were using. Changing the seed seems to resolve the issue.
  • There used to be a check on the norm of the terms being added to a LocalOperator, where any term whose norm was too small was just skipped. This is no longer present in the add_term! implementation, causing a failure of @assert length(imag(H).terms) == 0 for a Hamiltonian with purely real terms. The cutoff was chosen a bit arbitrarily, but I think it can't hurt to just to skip terms whose norm is exactly zero, i.e, using an atol = zero(real(scalartype(term))). I can add that.
    • UPDATE: I added an atol check for the norm of a term in add_term!, and set the default to only skip terms with an exact zero norm.
  • It looks like a SpaceMismatch snuck into the time evolution workflow. No idea where this is coming from exactly, but I'll try to investigate a bit more.

@Yue-Zhengyuan
Copy link
Copy Markdown
Member

I'll try to fix the time evolution part.

@Yue-Zhengyuan Yue-Zhengyuan self-assigned this Apr 2, 2026
@Yue-Zhengyuan
Copy link
Copy Markdown
Member

The space mismatch is because the site order for gates on vertical bonds becomes (north, south) (due to how Julia sorts CartesianIndex's), which is opposite to the "canonical order" (along the virtual arrow directions) in iPEPS/PEPO. The produced bond weight should then be transposed, which was missing before.

I added more careful checks for MPO gates in LocalCircuit:

  • Two consecutive sites along the path inds should be nearest neighbors. For this reason, we should not sort inds for MPO terms.
  • inds cannot contain repeated coordinates (for LocalOperator as well).

I also removed the check that H and the Trotterized circuit should have the same number of terms. For example, each NNN term is converted to two MPO gates.

1        1---2
|            |
2---3        3

@leburgel I guess you can now proceed to see if there are other things to be cleaned up.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Your PR no longer requires formatting changes. Thank you for your contribution!

@pbrehmer
Copy link
Copy Markdown
Collaborator

pbrehmer commented Apr 2, 2026

This is really nice, thanks! I update the docs and model definitions to define the LocalOperator terms using Vectors instead of NTuples since that is now more in line with the field types (although the tuple definitions still work because add_term! just needs in iterable type I guess).

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.

4 participants