Bug & suggestions for expectation_computational_basis_state

[vincentelfving]

I noticed an incorrect behavior, afaik, of the utils._sparse_tools function expectation_computational_basis_state.
Shortest reproduction of such an error:

operator = normal_ordered(FermionOperator('0^ 0', 1.9) +
                                            FermionOperator('2^ 1') +
                                            FermionOperator('0^ 1^ 0 1', -1.7) + 
                                            FermionOperator('2^ 0^ 2 0', -11.7))
state = [1, 1, 0]

print(expectation_computational_basis_state(operator,state))

returns 15.3 instead of 3.6 which we'd expect. The problem occurs only for the double excitations operators. The test which was supposed to catch such an error for this function, happened to pass by coincidence due to the construction of that specific task.

Suggested solution: In utils._sparse_tools function expectation_computational_basis_state, between L836 and L837 there should be a check for orbital j whether it is in the list occupied_orbitals or not, i.e. add an if occupied_orbitals[j]:-statement such that:

for j in range(i+1, len(occupied_orbitals)):
    if occupied_orbitals[j]:
        expectation_value -= operator.terms.get(
                                     ((j, 1), (i, 1), (j, 0), (i, 0)), 0.0)

Also, a suggestion is to assert operator.is_normal_ordered() within the body of the function expectation_computational_basis_state, because (as it correctly states in the requirements of the args) the function only works properly if the FermionOperator is normal-ordered. I'm not sure if it should raise an error or automatically apply normal_ordering() to the operator...

Also, a suggestion is to change the Args description of this function. Currently, it says '..or list of occupied orbitals.' it is not immediately obvious if the user should enter, say, [0,2,3] or [1,0,1,1,0,0] (it should be the latter, for this function)

Another note: this function assumes an input FermionOperator is always a quadratic hamiltonian (with at most two excitations) otherwise it does not work, but I think that's ok.

[babbush]

Thanks for catching this Vincent. I agree with your proposed change. Rather than add the assertion, one should check to make sure it is true, and if not raise an exception that can make the problem clear to the user.

[S-Erik]

I fixed this in PR #1194. Thanks @vincentelfving for doing the implementation sketch.

[mhucka]

Fixed by PR #1194.