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

Add tests to CI #24

Merged
merged 21 commits into from
May 16, 2024
Merged

Add tests to CI #24

merged 21 commits into from
May 16, 2024

Conversation

lkdvos
Copy link
Collaborator

@lkdvos lkdvos commented May 14, 2024

This moves some of the tests that were in the examples folder to a proper testing framework

@lkdvos lkdvos requested a review from pbrehmer May 14, 2024 09:07
@lkdvos
Copy link
Collaborator Author

lkdvos commented May 14, 2024

This seems like its still not very stable, and we should probably do something about the overly verbose printing... I might be able to find some more time this afternoon

@pbrehmer
Copy link
Collaborator

I will also take a look in a bit, maybe I manage to stabilize the tests.

@pbrehmer
Copy link
Collaborator

I like how MPSKit prints out the first and last VUMPS iterations when you set verbosity=1. I think this would be useful to have also in PEPSKit and it would give some useful printing during the tests. Let me quickly add that, while I'm at it!

@lkdvos
Copy link
Collaborator Author

lkdvos commented May 14, 2024

I definitely agree, sadly the machinery for that is still in the untagged master of mpskit, so its not so easy to just import it :p

pbrehmer
pbrehmer previously approved these changes May 14, 2024
@pbrehmer
Copy link
Collaborator

Maybe, once it's tagged, we can use that machinery to make the output printing and logging a bit more nice :)

@pbrehmer
Copy link
Collaborator

pbrehmer commented May 15, 2024

Well, the tests ran through on v1.9.3, let's see if I can also get them to run on v1.6.

Update: The gauge-fixing failed (for certain seeds) because the environment dimension was too low - increasing it solved it. I don't get why the success of the gauge fixing should be dependent on the environment dimension? (The error was $\mathcal{O}(1)$ and not just a bit inaccurate, so something failed completely...)

@pbrehmer
Copy link
Collaborator

@lkdvos Do you think we could get rid of the Julia 1 tests? I'm not sure if it makes sense to be backwards compatible to v1.0 if PEPSKit anyway still is in a very experimental stage. Also it seems to be difficult to get the tests running consistently on both v1.6 and v1.0. Maybe we could instead test on the latest stable release.

@lkdvos
Copy link
Collaborator Author

lkdvos commented May 15, 2024

I think the Julia 1 tests expand to the latest stable Julia, so 1.10 in this case...

@pbrehmer
Copy link
Collaborator

Whoops, that's good to know and definitely makes more sense. I'll try again tomorrow to get the tests running consistently (and maybe to speed them up, so the CI doesn't take ages...)

@lkdvos
Copy link
Collaborator Author

lkdvos commented May 15, 2024

hahahhaha, so now I've fixed v1 but broke 1.6...

@pbrehmer
Copy link
Collaborator

It's really annoying that some of these runs are so sensitive (at low bond/environment dimension) to the random initial state. I guess it takes a bit of trying around with the random seed to find one that works for both versions. I can try around tomorrow if you don't have the time/motivation now (which would be perfectly understandable :D)

@lkdvos
Copy link
Collaborator Author

lkdvos commented May 15, 2024

So at this point, I'm basically ready to give up...
Probably it is better to just look for a reasonable physical state somewhere, such as some state which is at the very least approximately close to a groundstate?
I'm fine with merging like this, and maybe we can add an ED thing on a 3x3 lattice later and then just decompose that state and use the center peps tensor?

@pbrehmer
Copy link
Collaborator

Thanks for the effort, it really is annoying! Having an ED initial PEPS also sounds like a good idea. I will give it a final go tomorrow but then I'd also be fine with merging.

@lkdvos
Copy link
Collaborator Author

lkdvos commented May 16, 2024

Small FYI, I tried playing around with the Ising PEPS, also does not seem to work (doesnt even want to converge with CTMRG). I think that one might also just be a bit pathological anyways, or I might have implemented it wrong, but I am kind of running out of ideas. I would think the best approach is to just save a reasonable state on disk somewhere, and use that

@pbrehmer
Copy link
Collaborator

I would think the best approach is to just save a reasonable state on disk somewhere, and use that

Yes, I agree that this is the most sensible approach in the long run. Maybe we can code up a small state/environment saving and loading interface soon. I guess that's useful to have anyways.

@pbrehmer
Copy link
Collaborator

Hope that this runs for now... I think this should be ready to merge!

@lkdvos lkdvos merged commit a6afe15 into master May 16, 2024
11 checks passed
@lkdvos lkdvos deleted the ld-tests branch May 16, 2024 12:50
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.

None yet

2 participants