-
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
Add Form
-specific constructors for MPS
#248
Conversation
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.
Great! Thanks! In general everything is great. Just some suggestions for the generic MPS
constructors.
Regroup repeated code into an internal function. Maybe it can be named
__tn_mps_from_arrays
?
You no longer need to prefix internal functions with __
since we'll soon start annotating public API with the new public
keyword (well, with the @public
macro from Compat.jl).
Also, what I would recommend you is not to refactor it to an internal function, but to leave that code in the NonCanonical
constructor. Then Canonical
and MixedCanonical
would call the NonCanonical
constructor and modify it.
MixedCanonical
calls theNonCanonical
constructor and then modifies theform
field.Canonical
calls theNonCanonical
constructor, tensorizes the alphas by requesting bond index name withinds(tn; bond=...)
, push them to the TN and finally modify theform
field.
An example of this pattern can be found in simple_update!
for MixedCanonical
: it first calls mixed_canonize!
to move the orthogonality center to the target sites and then calls NonCanonical
version of simple_update!
.
Okay, everything should be ready now @mofeing. I separated the checks of the form in two |
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.
Great! Just some suggestions and we're done.
src/MPS.jl
Outdated
function check_form(::Canonical, mps::AbstractMPO) | ||
for i in 1:nsites(mps) | ||
if i > 1 | ||
isisometry(contract(mps; between=(Site(i - 1), Site(i)), direction=:right), Site(i); dir=:right) || |
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.
do you need to contract to check? that can be a lil bit costly
@starsfordummies what do you think? is there some other way to check that the gammas are correct?
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'm okay with merging it this like for now. But if there's a way to check it without contracting, then we should open an issue to implement it in the future.
Thanks! |
This PR adds
Form
-specific constructors for theMPS
struct, which allows us to create Matrix Product States with an specific canonicalForm
directly on the constructor. I believe this is the proper way to go, and we added acheck_canonical_form
kwarg
that, iftrue
(default), it will check if theMPS
fulfills the canonical form.TO DO:
Regroup repeated code into an internal function. Maybe it can be namedGroup code by using__tn_mps_from_arrays
? @mofeing I can't find a good name.NonCanonical
constructor.