Closed Strilanc closed 7 years ago
So it's kind of funny you suggest this because we used to have exactly this implemented. But after a long discussion we decided to get rid of the LadderOperator. There are a number of reasons related to performance basically. This would also effect a huge amount of code. For now, I am going to close this issue.
What kind of performance reasons? I have a hard time picturing why there'd be much difference.
Well for FermionOperator it might not introduce real performance issues, but for QubitOperator, I think it would. This is related to the fact that in QubitOperator we need to keep the pauli operators sorted by tensor factor in order to perform operations like multiplication quickly. I think that if the ladder operators are more complicated classes, this is going to effect performance. And it is very important to us that we keep the parallel structure between FermionOperator and QubitOperator.
But thinking more about this, the bigger issue I have, one I feel very strongly about, is that this is going to introduce changes that effect huge parts of the library and possibly make the library more complicated and I do not see the benefit. ((p, 1), (q, 0), (r, 1), (s, 0)) is more intuitive to me and resembles our physics equations rather well. Also, I never see a reason why we'd want to create the term by multiplying ladder operators like this. Let's talk this week but I just think our time would be better spent elsewhere. There is plenty of work to do still!
FermionOperator has a terms field with a complicated dict-of-tuple-of-tuple-to-complex type.
The dictionary's key type should be a class
LadderOperators
. AndLadderOperators
should be a collection ofLadderOperator
instances. Both should support arithmetic operators for combining them, so that for example you could writep/q*r/s
instead of((p, 1), (q, 0), (r, 1), (s, 0))
.