Enhance mps/mpo reproducibility; Print git hash for easier debugging;… #162
Conversation
jiangtong1000
commented
Sep 13, 2023
•
jiangtong1000
commented
- Enhance mps/mpo reproducibility; Currently, even with preset random seed, the MPO or the subsequent ground state MPS will be different for two runs. The MPO is different because the Op order can be different before doing Hopcroft-Corp; The MPS can be different because the sign carried by the eigenvectors. This does not influence the final energy, but keeping everything reproducible could be important at sometime, especially for debugging purpose
- The git hash information is printed every time running the package, for easier debugging
- Fix mp dump/load
… fix mp dump/load
jiangtong1000
force-pushed
the
reproducible
branch
from
8640710
to
1cfa0ec
Compare
|
I really like this commit but there are some technical problems. Please see the comments |
renormalizer/mps/gs.py
Outdated
| @@ -383,10 +383,10 @@ def eigh_direct( | |||
| nroots = mps.optimize_config.nroots | |||
| if nroots == 1: | |||
| e = w[0] | |||
| c = v[:, 0] | |||
| c = v[:, 0] / np.sign(np.max(v[:, 0])) | |||
There was a problem hiding this comment.
I don't think this works as expected. You might first want to take absolute values before calling max. Also, this "canonicalize" logic is used below. Maybe it's better to wrap it as a utility function
There was a problem hiding this comment.
util function added and bug fixed
renormalizer/mps/gs.py
Outdated
| if nroots == 1: | ||
| return e, c/np.sign(np.max(c)) | ||
| else: | ||
| return e, c/np.sign(np.max(c, axis=0)) |
There was a problem hiding this comment.
This might cause the bug in CI. Taking max along the 0th axis will eliminate the 0th axis, resulting a vector with size N, where N is the dimension of c.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #162 +/- ##
=======================================
Coverage 84.79% 84.80%
=======================================
Files 105 105
Lines 10184 10207 +23
=======================================
+ Hits 8636 8656 +20
- Misses 1548 1551 +3
☔ View full report in Codecov by Sentry. |
renormalizer/mps/gs.py
Outdated
| def sign_fix(c, nroots): | ||
| if nroots > 1: | ||
| if isinstance(c, list): | ||
| return [ci / np.sign(np.max(np.abs(ci))) for ci in c] |
There was a problem hiding this comment.
- Taking abs loses the sign information
- Add abs for the following branches as well?
There was a problem hiding this comment.
- yea you are right. my mistake
- what do you mean the following branches?
There was a problem hiding this comment.
when c is not a list and when nroots == 1
renormalizer/mps/gs.py
Outdated
| else: | ||
| return c / np.sign(np.max(c, axis=0)) | ||
| return c / np.sign(np.max(np.abs(c), axis=0)) |
There was a problem hiding this comment.
It seems in this branch it still does not work properly?
There was a problem hiding this comment.
thanks for pointing out! Now I think it is resolved.
