-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor Ansatz
to be a concrete type with lattice information
#223
Conversation
@mofeing @starsfordummies I think that when we talked about splitting the large PR into small ones, we meant that this PR would only contain the new |
Sorry, but I don't agree. The The other ansatzes will be reimplemented in subsequent PRs because tests had to be changed. |
7a870b7
to
0bef4a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to have @test_skip
instead of commenting all the tests.
That's a valid request and a good idea. Will change it now and recover the deleted tests as |
…top of recent changes
Co-authored-by: Jofre Vallès Muns <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
If we fix the documentation I guess we can merge |
Docs are basically broken. We need to redo them but after this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sergio breaks tenet in all possible ways without even documenting what's broken. Typical commit, looks good to me 👍
* Refactor `Product` on top of new `Ansatz` type * Format code * Fix typo * Refactor `adapt_structure` method to support additional types * Refactor `Reactant.make_tracer`, `Reactant.create_result` methods on top of recent changes * Refactor `ChainRules` methods on top of new types * Aesthetic name fix * Stop using `IdDict` on Reactant extension * Refactor lattice generation in constructors of `Dense`, `Product`, `MPS`, `MPO`, `PEPS` * Reenable `Product` tests * fix constructors * remove `zeros`, `ones` (not well defined) * fix symbol import * export `Product` * fix constructors * refactor tests * fix typo * move file * Document `Product` constructor
As discussed offline, we decided to slice #204 into smaller PRs. This first PR makes
Ansatz
a concrete type with lattice information contained in a graph.The important thing you should review is the API, not the details; we can change the graph type in the future but we want the API to be stable.
Changelog
Lattice
graph typeGraphs.SimpleGraph
with a bijective mapping between vertices andSite
sGraphs
andas dependencMetaGraphsNext
iesyAnsatz
a concrete type in the type hierarchyAbstractAnsatz
Chain
,Product
,Grid
andDense
Ansatz
layerForm
the canonical form information traitevolve!
methods based on simple-update tosimple_update!
and generalize it for anyAbstractAnsatz
@testset_skip
macro to implement @jofrevalles's idea of skipping whole testsets temporarilyNotes
Currently there is a bug in Julia makes Enzyme crash on anShould no longer be necessary because we replacedAnsatz
due to its fieldlattice
being abstractactive_reg_inner
crashes onTuple{X} where {X}
EnzymeAD/Enzyme.jl#1923MetaGraph
forLattice
It's fixed in Julia 1.11.0 and should be fixed in Julia1.10.61.10.7I temporarily fixed it by fixingAnsatz
to just 1D connectivity (i.e.Site{1}
) but I'm removing it now -> For the time being you will need Julia 1.11 if you want to diff anAnsatz
There are other bug fixes (likeAlready pushed to masterBase.in
orGraphMakie.graphplot!
onAbstractTensorNetwork
) that I'm gonna remove now and move to other PRsLattice
toTopology
in the future, because it's more accurate if we're not using the partial ordering property of latticesLattice
for custom ones to accommodate clustered sites (i.e. a tensor containing 2+ sites) or hypergraphs