Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concretize Ansatz type with "lattice"/connectivity information and refactor subtypes on top of it #204

Draft
wants to merge 75 commits into
base: master
Choose a base branch
from

Conversation

mofeing
Copy link
Member

@mofeing mofeing commented Sep 16, 2024

Inspired by ITensorNetworks.jl (but not equal), I experimented with using a graph for representing the connectivity between different sites.

In "ITensorNetworks.jl", they use the graph for representing the structure of the TensorNetwork and the connectivity graph of the different sites, which is useful when dealing with lattices or hamiltonians. Unlike them, the hypergraph structure is already implicit in TensorNetwork, but it doesn't comfortably accommodate for fixed structured Tensor Networks like MPS, PEPS, TTN, ...

By adding a graph field in Ansatz that accounts for the connectivity pattern between sites, a lot of the software design problems can be mitigated.

I've also refactored Chain into MPS and MPO and Grid into PEPS and PEPO (actually, I've never seen a PEPO in real life).

@jofrevalles @starsfordummies you might want to reconsider refactoring your PR on top of this.

@mofeing mofeing added refactor Change internals triage Needs group consensus breaking-change labels Sep 17, 2024
@mofeing mofeing self-assigned this Sep 17, 2024
@mofeing mofeing force-pushed the refactor/ansatz-lattice branch 4 times, most recently from 678c3af to 9b22b13 Compare September 19, 2024 17:55
@mofeing mofeing added the on hold Blocked due to some reason label Sep 23, 2024
@mofeing
Copy link
Member Author

mofeing commented Sep 23, 2024

@jofrevalles Seems like the problem is (again) related to the way indices are replaced when doing merge!. I thought it was a good idea, but we might want to rethink how to deal with indices on that case.

@jofrevalles
Copy link
Member

@jofrevalles Seems like the problem is (again) related to the way indices are replaced when doing merge!. I thought it was a good idea, but we might want to rethink how to deal with indices on that case.

In my opinion, it would be much better to revert the code to what we had previously (so indices are not repeated, and just have an uuid). If we want to have "nice symbols" when doing plots, we can do some conversion or something before plotting in order to fix the labels.

@mofeing
Copy link
Member Author

mofeing commented Sep 23, 2024

If we want to have "nice symbols" when doing plots, we can do some conversion or something before plotting in order to fix the labels.

I wouldn't change the indices for plotting because it will lie to the users.

@jofrevalles
Copy link
Member

I wouldn't change the indices for plotting because it will lie to the users.

I mean, you can take any tn, and replace the indices with any mapping you want. For example, we could modify the internals so you could choose to use uuid for some situations, and "nice symbols" (using some kind of index counter as we had) in other situations.

mofeing and others added 22 commits September 26, 2024 12:33
…nsors) (#218)

* Format code

* Implement MPS identity initialization

* Add tests for all dispatches of MPS identity init

* Format julia code

* Rename function header & add docstring

* Fix test set for identity MPS

* Format code

* Rewrite MPS identity init function to nsites instead of arrays' dimensions

* Format julia code

* Update docstring of identity

* Clean code in test (suggested by Jofre)

Co-authored-by: Jofre Vallès Muns <[email protected]>

* Format julia code

* Refactor virtualdims in identity (suggested by Sergio)

* Update src/Ansatz/MPS.jl

* Restrict to default order in identity MPS

* Update src/Ansatz/MPS.jl

* Remove order parameter in identity

---------

Co-authored-by: Jofre Vallès Muns <[email protected]>
Co-authored-by: Sergio Sánchez Ramírez <[email protected]>
Fixes some problems when calling `make_tracer` on an object that wraps an `Ansatz`, like `Tuple{MPS}` for example.
@mofeing mofeing marked this pull request as ready for review October 21, 2024 10:17
@mofeing mofeing marked this pull request as draft November 3, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change on hold Blocked due to some reason refactor Change internals triage Needs group consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Ansatz type-hierarchy to a trait system
3 participants