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

Incorporated an alg argument for GD which can be set to SDD(fast, unstable) or SVD(slow, stable) #175

Closed Gertian closed 2 months ago

Gertian commented 3 months ago

Together with https://github.com/Jutho/TensorKitManifolds.jl/pull/11 this implements functionality that allows the user to choose which SVD algorithm is being used internally in the gradient descent gradient calculation.

The options are SDD(fast but potentially unstable) vs SVD(slow but stable).

These two PR should fix the issue : https://github.com/QuantumKitHub/MPSKit.jl/issues/109

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Files with missing lines Coverage Δ
src/algorithms/grassmann.jl 100.00% <100.00%> (+0.86%) :arrow_up:
src/algorithms/groundstate/gradient_grassmann.jl 80.00% <100.00%> (+3.07%) :arrow_up:
lkdvos commented 3 months ago

I think you swapped SDD (fast, unstable) with SVD (slower, stable). Can you try and run the formatter as well?

Gertian commented 3 months ago

@lkdvos I updated everything according to your input. Somehow the Format suggestion test keeps failing. Despite me formatting all files I touched.

Gertian commented 3 months ago

@lkdvos I updated everything according to your input. Somehow the Format suggestion test keeps failing. Despite me formatting all files I touched.

Ok, this has been resolved. For later reference, I had to make sure VSCode had the correct workspace loaded (so that .JuliaFormatter.toml was in my working directory) and then just run the normal formatter :)

lkdvos commented 3 months ago

If you want, you can always manually run the formatter on the entire repository:

using JuliaFormatter
format("src")
Gertian commented 2 months ago

@Jutho , @lkdvos what is the status here ? Can this and the corresponding TensorKitManifolds PR https://github.com/Jutho/TensorKitManifolds.jl/pull/11 be merged ? Or is there still work to be done ?

Jutho commented 2 months ago

Again, apologies for the late response here too. With my suggestions for the TensorKitManifolds.jl, I don't think this is necessary anymore. I agree this approach is nicer, and so maybe this PR can stay open, but this only makes sense after a larger TensorKitManifolds.jl overhaul. As mentioned TensorKitManifolds.jl, there are also different retraction schemes possible for Grassmann, and the alg keyword of retract should be used for that, and might then be configurable in the GrassmannMPS algorithm.

Gertian commented 2 months ago

Ok. I personally don't mind the specific implementation as long as I have access to the stable SVD through some option in either MPSKit or TensorKitManifolds :+1:

lkdvos commented 2 months ago

Given the recent discussions, I will close this for now. There is definitely some clean up work that can be done for the entire Grassman part of the code, but this will probably be for a different PR...