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

[ITensorMPS] Make ITensorTDVP functions into compatibility layer #1371

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

emstoudenmire
Copy link
Collaborator

This PR changes the names of the interface functions moved from ITensorTDVP. Now they are prepended with itensortdvp_ so as to be a compatibility layer behind which we can translate arguments to be able to work on the ITensorMPS.alternating_update code however we want without breaking ITensorTDVP.

@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 50.30%. Comparing base (f06d5c7) to head (dc4d504).

❗ Current head dc4d504 differs from pull request most recent head 3fc6cbc. Consider uploading reports for the commit 3fc6cbc to get more accurate results

Files Patch % Lines
src/ITensorMPS/tdvp.jl 0.00% 12 Missing ⚠️
src/ITensorMPS/update_observer.jl 0.00% 3 Missing ⚠️
src/ITensorMPS/contract_mpo_mps.jl 0.00% 1 Missing ⚠️
src/ITensorMPS/dmrg.jl 0.00% 1 Missing ⚠️
src/ITensorMPS/dmrg_x.jl 0.00% 1 Missing ⚠️
src/ITensorMPS/linsolve.jl 0.00% 1 Missing ⚠️
src/ITensorMPS/sweep_update.jl 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
- Coverage   57.58%   50.30%   -7.28%     
==========================================
  Files         114      113       -1     
  Lines        9004     8965      -39     
==========================================
- Hits         5185     4510     -675     
- Misses       3819     4455     +636     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtfishman
Copy link
Member

mtfishman commented Apr 2, 2024

@emstoudenmire I also see this warning:

WARNING: Method definition update!(ITensors.ITensorMPS.AbstractObserver) in module ITensorMPS at /Users/mfishman/.julia/packages/ITensors/jmIjr/src/ITensorMPS/update_observer.jl:3 overwritten in module ITensorTDVP at /Users/mfishman/.julia/dev/ITensorTDVP/src/update_observer.jl:4.

because of the overload here: https://github.com/ITensor/ITensors.jl/blob/v0.3.60/src/ITensorMPS/update_observer.jl.

Let's just remove that for now and decide how update! will work in a future PR.

Removing that overload aligns with the plan of moving the Observers.jl dependency to a package extension anyway, since that will likely be implemented by defining a new function (say called ITensorMPS.update_observer!) which will be used in alternating_update and can be overloaded on Observers.Observer (in an ITensorsObserversExt package extension) and ITensorMPS.AbstractObserver.

EDIT: We could already start that process here, by defining an interface function ITensorMPS.update_observer! with an empty definition, and overload it for ITensorMPS.AbstractObserver. Then when we make ITensorsObserversExt in a future PR it can just overload ITensorMPS.update_observer! on Observers.Observer.

@mtfishman
Copy link
Member

[test ITensorMPS][test ITensorGaussianMPS]

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Run ITensorGaussianMPS tests from comment trigger: succeeded ✅
https://github.com/ITensor/ITensors.jl/actions/runs/8528892800

1 similar comment
Copy link
Contributor

github-actions bot commented Apr 2, 2024

Run ITensorGaussianMPS tests from comment trigger: succeeded ✅
https://github.com/ITensor/ITensors.jl/actions/runs/8528892800

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Run ITensorMPS tests from comment trigger: succeeded ✅
https://github.com/ITensor/ITensors.jl/actions/runs/8528892802

1 similar comment
Copy link
Contributor

github-actions bot commented Apr 2, 2024

Run ITensorMPS tests from comment trigger: succeeded ✅
https://github.com/ITensor/ITensors.jl/actions/runs/8528892802

@mtfishman mtfishman merged commit 9f1ee2a into main Apr 2, 2024
11 checks passed
@mtfishman mtfishman deleted the itensortdvp_compatibility_layer branch April 2, 2024 20:44
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