Remove MPSKitModels dependency#394
Conversation
- Replace all MPSKitModels operator building blocks with TensorKitTensors counterparts (SpinOperators, BosonOperators, HubbardOperators, TJOperators) - Convert model constructors from MPSKitModels extensions to PEPSKit-native functions - Remove AbstractLattice supertype; InfiniteSquare is now a plain struct - Update tests, examples, docs, and notebooks to use TensorKitTensors imports - Remove MPSKitModels from all Project.toml files Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
Definitely in favor of using TensorKitTensors for the operators, but I'm not entirely sure if we gain a lot by decoupling the actual models. The main thing I worry about is that there are now two functions for each model, and you have to remember that PEPSKit.hubbard_model !== MPSKitModels.hubbard_model.
We should however probably discuss this in a bit more detail together, since I agree that the MPSKitModels package is a bit awkward. I would be happy to include most of that functionality in MPSKit, for example, since I feel like it does not actually warrant a separate package, but in that case probably we should move out the lattice stuff in a different package (ie. TiledArrays/LatticeArrays or something similar).
This is easy 😂 Actually for newcomers I don't think they expect MPSKitModels to provide 2D Hamiltonians? |
|
Well, the point is that if you accidentally do something like |
Before TensorKitTensors exists, I needed to import MPSKitModels to access pre-defined operators in order to define custom Hamiltonians. This was already causing a name clash on
Dependence on MPSKit is quite reasonable, since it should be common knowledge that PEPS can be contracted with boundary MPS methods. But getting PEPSKit Hamiltonians from MPSKitModels has always weirded me out. After rewriting the Hamiltonians with TensorKitTensors, what remains that still depends on MPSKitModels is just the AbstractLattice stuff. Since we are currently supporting square lattice only, I think it's not worth it to keep depending on another package that is not designed to work with PEPS. Although this will be a breaking change, the function signatures remain untouched, so the difference is not that dramatic. |
This PR removes MPSKitModels dependency. The main purpose is to use pre-defined operators from TensorKitTensors.
AbstractLatticeis lost. But I think this will eventually be replaced by TiledArrays?Some issues that are addressed:
sublattice = true(S_xx, S_yy with U1Irrep symmetry MPSKitModels.jl#57). With TensorKitTensors, an error will be correctly raised.pwave_superconductoris actually a p-ip (instead of p+ip) wave Hamiltonian. I wonder if we want to change the convention and update the benchmark energy in the test.