QuantumKitHub / MPSKit.jl

A Julia package dedicated to simulating quantum many-body systems using Matrix Product States (MPS)
MIT License
139 stars 30 forks source link

Combine pr #182

Closed Gertian closed 1 month ago

Gertian commented 1 month ago

In case it's more convenient for the main devs (I guess @lkdvos ), this PR merges my three prior PRs into one.

So in this PR I've combined 1) parallel GD 2) parallel VumpsSvd 3) tests for n-site unit cells

Most notably the new tests seemed important for me since the parallel GD and VumpsSvd might only fail when multiple sites are present and the parallel code kicks in...

At the very least this PR allows the reviewer to see that the new code works with the new tests :)

It's equivalent to merge this PR or merge the 3 previous PRs that I just did :) Like I said, whatever is most convenient...

lkdvos commented 1 month ago

I'm gonna do my best to get to all of this and look at everything as soon as possible, I'm a bit short on time these days. (Moving houses, traveling and preparing presentations seem to be taken up a lot of time...) As a small side-note, I wanted to ask how urgent this is. I do agree we want to add is, but we have an upcoming major rewrite in the branch blocktensor2, which is more or less working but requires an updated version of tensorkit first, which can then release blocktensorkit, which then makes that pr mergable. I am a bit hesitant to make many intermediate changes to avoid having to port over too much of the code, but I also see that it is not reasonable to completely stop development in the meantime, so it's all about striking a balance. Anyways, if I still didn't get to this by end of next week, feel free to ping me again!

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 84.21053% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/algorithms/grassmann.jl 76.19% 10 Missing :warning:
src/algorithms/changebonds/vumpssvd.jl 94.11% 2 Missing :warning:
Files with missing lines Coverage Δ
src/algorithms/changebonds/vumpssvd.jl 97.22% <94.11%> (+25.42%) :arrow_up:
src/algorithms/grassmann.jl 92.95% <76.19%> (-7.05%) :arrow_down:

... and 4 files with indirect coverage changes

Gertian commented 1 month ago

Hi,

Thanks for the quick reply.

I wouldn't say its super urgent, but on the other hand, for my usecase with unit cell 32 this should amount to a HUGE speed increase.

I guess I can always use my local branch though. I don't like doing this though.

Ps : a lot of the commits are me forgetting to format. Or fixing typos. The actual amount of new code is quite minimal.

Gertian commented 1 month ago

@lkdvos seems like you've already reviewed the other PRs so this is irrelevant now.

As you suggested the best way forward is probably to first merge the tests and then the other two PRs !