Feature/sweep bounds #26
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #26 +/- ##
==========================================
Coverage ? 87.96%
==========================================
Files ? 31
Lines ? 7720
Branches ? 0
==========================================
Hits ? 6791
Misses ? 929
Partials ? 0
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for this @pulkin, looks useful!
Aside from the minor comments above, a few more general points:
- The
boundsspecification might be made a bit clearer. Currently it says inclusive sites at one point but exclusive elsewhere. My preference would be ifbounds=(s, e)meant optimize all sites inrange(s, e)-- so not dependent onbszand thus the same arg could be used for bothDMRG1andDMRG2. - Could you lint the code for PEP8 (>80 lines and spaces around operators etc.)
quimb/tensor/tensor_dmrg.py
Outdated
| @@ -866,24 +866,39 @@ def sweep(self, direction, canonize=True, verbosity=0, **update_opts): | |||
| Sweep from left to right (->) or right to left (<-) respectively. | |||
| canonize : bool, optional | |||
| Canonize the state first, not needed if doing alternate sweeps. | |||
| bounds : tuple, optional | |||
| A 2-tuple with cites (from and to, not included) to perform the sweep | |||
There was a problem hiding this comment.
minor spelling: 'cites' -> 'sites'
| p0 = MPS_rand_state(5, 1) | ||
| p0[0].data[:] = down().T | ||
|
|
||
| dmrg = DMRG2(H_mpo, p0=p0) |
There was a problem hiding this comment.
Might be good to parametrize over DMRG1 and DMRG2.
quimb/tensor/tensor_dmrg.py
Outdated
| bounds = bounds_ext | ||
|
|
||
| if (direction == "right" and bounds[0] >= bounds[1]) or (direction == "left" and bounds[0] <= bounds[1]): | ||
| raise ValueError(f"Invalid bounds for direction={direction} (wrong order): {bounds}") |
There was a problem hiding this comment.
Although f-strings are really nice, it would be good to stick to .format syntax for the moment, since notionally quimb should still support python 3.5.
|
Just to clarify: given |
|
So, my feeling is that the sweep should just be: range(s, e - bsz + 1) # for direction 'R', or
reversed(range(s, e - bsz + 1)) # for direction 'L'So if you wanted to keep a single site held, in either dmrg.solve(bounds=(0, mysite))
dmrg.solve(bounds=(mysite + 1, n))
...What do you think? |
|
So there are some subtle complexities that require a bit more work here.
It might be easier for me to address these just as I know the code better, but also happy for you to try! Let me know, I'd need commit permission on this PR. |
A possibility to limit bounds of sweeps in DMRG1,2. Not tested for loop networks.