Skip to content

CompatHelper: bump compat for MPSKit to 0.12, (keep existing compat)#119

Merged
leburgel merged 31 commits intomasterfrom
compathelper/new_version/2025-01-23-01-10-02-510-04226771641
Jan 28, 2025
Merged

CompatHelper: bump compat for MPSKit to 0.12, (keep existing compat)#119
leburgel merged 31 commits intomasterfrom
compathelper/new_version/2025-01-23-01-10-02-510-04226771641

Conversation

@github-actions
Copy link
Contributor

This pull request changes the compat entry for the MPSKit package from 0.11 to 0.11, 0.12.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

@codecov
Copy link

codecov bot commented Jan 23, 2025

@Yue-Zhengyuan Yue-Zhengyuan mentioned this pull request Jan 24, 2025
5 tasks
@pbrehmer
Copy link
Collaborator

This seems to work now, except for all the boundary MPS things. Note that we here update MPSKit, MPSKitModels, KrylovKit, VectorInterface, TensorKit and TensorOperations at once since that way it was easier to disentangle all the compat requirements.

Once we get the boundary MPS things going, I'll take a look at the Zygote compat PR and merge that ASAP.

@leburgel
Copy link
Member

leburgel commented Jan 27, 2025

This should be almost good to go I think. Boundary MPS contractions are running, and I took the liberty to reorganize and merge things as best I could right now. There's still some things I would like to do (e.g. update the contractions to use @autoopt, etc...), but nothing that should be blocking this.

The MPSKit updates allow to remove a bit of redundant code here (e.g. additional derivative structs such as PEPS_∂∂AC and the likes), but there's still quite a lot of things that had to be overloaded to make things work here, even though the actual 'interface' bits are quite compact. If we either

  • Remove the O<:MPOTensor restriction in the type parameter of the MPO types, so that we can define things like const InfiniteTransferPEPS{T<:PEPSTensor} = InfiniteMPO{Tuple{T,T}}
  • Systematically remove some type restrictions on the operators occurring in the MPSKit methods, such that we can just pass our own structs as long as they adhere to a minimal interface

then we can just remove most of the boundary MPS code here and things will still just work. But this requires some work on MPSKit in the future, for now this is good enough I think.

@leburgel leburgel requested a review from lkdvos January 27, 2025 20:14
Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I left a few minor comments, and it looks indeed like it will be a lot nicer once we lift some of the type restrictions in MPSKit, but I like getting this merged already in the meantime.

leburgel and others added 3 commits January 28, 2025 10:51
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for having a second round of comments, this really should be my final ones :).

I think I would still like to see the one remark about type piracy resolved, and restore the KrylovKit compat. Otherwise good to go!

Given the recent changes, and the restoration of compatibility. How do we feel about tagging this as a new (breaking) release?

leburgel and others added 3 commits January 28, 2025 12:36
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
lkdvos
lkdvos previously approved these changes Jan 28, 2025
Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If CI passes, good to go for me

@leburgel leburgel enabled auto-merge (squash) January 28, 2025 12:04
@leburgel leburgel requested a review from lkdvos January 28, 2025 12:18
@pbrehmer
Copy link
Collaborator

Given the recent changes, and the restoration of compatibility. How do we feel about tagging this as a new (breaking) release?

Sounds reasonable to me :-) But before that I would also like to bump the Zygote and OptimKit compat so that everything is up-to-date. I'll do that later today.

@lkdvos
Copy link
Member

lkdvos commented Jan 28, 2025

Fair warning that the OptimKit update is not possible yet, since that has to trickle through MPSKit first. If the Zygote update does not yield too much of a headache, I'm okay with getting that in before tagging a release, but fixing that should not be a breaking change.
I would really push on tagging a release sooner rather than later, so people can start working with released versions of the software again.

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.

3 participants