Typo in one-site Jordan AC Hamiltonian constructor - #493
Conversation
| end | ||
|
|
||
| @testset "Long-range Hamiltonian with real scalartype" verbose = true begin | ||
| # A real `H` optimised over a complex MPS sends the derivative operators through their |
There was a problem hiding this comment.
I'm not sure I'm understanding what this means to be honest, would you mind explaining?
There was a problem hiding this comment.
I think I explained it elsewhere better, but basically when the scalartype of the Hamiltonian and the state aren't equal at the real/complex level (I didn't test at the level of precision), JordanMPO_AC_Hamiltonian will go through the fallback outer constructor.
Two things had to happen at the same time. First, there needs to be an onsite term so the B-block exists, and second it needs to have a different scalartype to the environments. The B-block doesn't inherit the type of these environments in this particular case, as can be seen in:
So the typed inner constructor doesn't work, and it goes through the untyped one which wants to convert accordingly, which then errored due to the typo.
I think I wrote this poorly in the comments, so I can change this if it helps to remember why those tests exist.
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
This one took a while to debug since I don't really know how these Jordan MPOs work. I now understand a little better 😄
It turns out two things have to occur simultaneously for this to have been noticed. First, the scalartype of the Hamiltonian and the state must mismatch, such that the constructor of the AC Hamiltonian doesn't get fast-forwarded to the inner constructor but lands on the fallback one with the typo. Second, the Hamiltonian needed to not have the ending block while also have the finishing one. Since I was playing around with something semi-long range, I came across this, which makes sense. Nearest-neighbor Hamiltonians will always have B.
Coincidentally, we never caught this in the tests because we don't look at long-range Hamiltonians. So I added that to the tests, along with fairly extensive tests on the derivatives themselves directly. Some might be overkill, but better safe than sorry. Locally the performance of these long-range Hamiltonians is notably terrible (see #459)
I also discovered the changelog, so I decided to add to that :)