Repository: quantumlib/OpenFermion
Pull request #103 (2017-11-06 22:21:35)

Bug in Jordan Wigner ordering
One can convert a FermionOperator to a Jordan-Wigner encoded sparse operator by either directly calling `jordan_wigner_sparse`, or by first converting it to a QubitOperator using `jordan_wigner` and then calling `qubit_operator_sparse`. I think these should give the same result, but they don't:

```
In [4]: ferm_op = FermionOperator('1')

In [5]: qubit_op = jordan_wigner(ferm_op)

In [6]: ferm_op_sparse = jordan_wigner_sparse(ferm_op)

In [7]: qubit_op_sparse = qubit_operator_sparse(qubit_op)

In [8]: print(ferm_op_sparse)
  (0, 1)	(1+0j)
  (2, 3)	(1+0j)

In [9]: print(qubit_op_sparse)
  (0, 1)	(1+0j)
  (2, 3)	(-1+0j)
```

This bug arises from different conventions being used in `jordan_wigner` and `jordan_wigner_ladder_sparse`. In the first case, the string of Pauli Z's are placed at lower indices (i.e., with six modes, a_3 gets mapped to `Z Z Z Q_lower I I`), and in the second case, the string is placed at higher indices (i.e., a_3 gets mapped to `I I I Q_lower Z Z`). I fixed this bug by changing `jordan_wigner_ladder_sparse` to use the first convention.

kevinsung: Okay I made the changes. I added a more explicit test for the reverse JW.
googlebot: So there's good news and bad news.

:thumbsup: The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so.  Everything is all good there.

:confused: The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter.  We need to confirm that all authors are ok with their commits being contributed to this project.  Please have them confirm that here in the pull request.

*Note to project maintainer: This is a terminal state, meaning the `cla/google` commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.*

<!-- need_author_consent -->
babbush: Great catch, thanks Kevin!

Patch:
diff --git a/src/openfermion/transforms/_jordan_wigner.py b/src/openfermion/transforms/_jordan_wigner.py
index a0fcffd..172037c 100644
--- a/src/openfermion/transforms/_jordan_wigner.py
+++ b/src/openfermion/transforms/_jordan_wigner.py
@@ -23,6 +23,10 @@ def jordan_wigner(operator):
     """ Apply the Jordan-Wigner transform to a FermionOperator or
     InteractionOperator to convert to a QubitOperator.
 
+    Operators are mapped as follows:
+    a_j^\dagger -> Z_0 .. Z_{j-1} (X_j - iY_j) / 2
+    a_j -> Z_0 .. Z_{j-1} (X_j + iY_j) / 2
+
     Returns:
         transformed_operator: An instance of the QubitOperator class.
 
diff --git a/src/openfermion/utils/_sparse_tools.py b/src/openfermion/utils/_sparse_tools.py
index 1f64f62..84943bd 100644
--- a/src/openfermion/utils/_sparse_tools.py
+++ b/src/openfermion/utils/_sparse_tools.py
@@ -57,6 +57,10 @@ def kronecker_operators(*args):
 def jordan_wigner_ladder_sparse(n_qubits, tensor_factor, ladder_type):
     """Make a matrix representation of a fermion ladder operator.
 
+    Operators are mapped as follows:
+    a_j^\dagger -> Z_0 .. Z_{j-1} (X_j - iY_j) / 2
+    a_j -> Z_0 .. Z_{j-1} (X_j + iY_j) / 2
+
     Args:
         index: This is a nonzero integer. The integer indicates the tensor
             factor and the sign indicates raising or lowering.
@@ -65,19 +69,23 @@ def jordan_wigner_ladder_sparse(n_qubits, tensor_factor, ladder_type):
     Returns:
         The corresponding SparseOperator.
     """
+    parities = tensor_factor * [pauli_z_csc]
     identities = [scipy.sparse.identity(
-        2 ** tensor_factor, dtype=complex, format='csc')]
-    parities = (n_qubits - tensor_factor - 1) * [pauli_z_csc]
+        2 ** (n_qubits - tensor_factor - 1), dtype=complex, format='csc')]
     if ladder_type:
-        operator = kronecker_operators(identities + [q_raise_csc] + parities)
+        operator = kronecker_operators(parities + [q_raise_csc] + identities)
     else:
-        operator = kronecker_operators(identities + [q_lower_csc] + parities)
+        operator = kronecker_operators(parities + [q_lower_csc] + identities)
     return operator
 
 
 def jordan_wigner_sparse(fermion_operator, n_qubits=None):
     """Initialize a SparseOperator from a FermionOperator.
 
+    Operators are mapped as follows:
+    a_j^\dagger -> Z_0 .. Z_{j-1} (X_j - iY_j) / 2
+    a_j -> Z_0 .. Z_{j-1} (X_j + iY_j) / 2
+
     Args:
         fermion_operator(FermionOperator): instance of the FermionOperator
             class.


Test patch:
diff --git a/src/openfermion/transforms/_reverse_jordan_wigner_test.py b/src/openfermion/transforms/_reverse_jordan_wigner_test.py
index 74342ea..ff0cf99 100644
--- a/src/openfermion/transforms/_reverse_jordan_wigner_test.py
+++ b/src/openfermion/transforms/_reverse_jordan_wigner_test.py
@@ -177,3 +177,12 @@ class ReverseJWTest(unittest.TestCase):
     def test_bad_type(self):
         with self.assertRaises(TypeError):
             reverse_jordan_wigner(3)
+
+    def test_jw_convention(self):
+        """Test that the Jordan-Wigner convention places the Z-string on
+        lower indices."""
+        qubit_op = QubitOperator('Z0 X1')
+        transformed_op = reverse_jordan_wigner(qubit_op)
+        expected_op = FermionOperator('1^')
+        expected_op += FermionOperator('1')
+        self.assertTrue(transformed_op.isclose(expected_op))
diff --git a/src/openfermion/utils/_sparse_tools_test.py b/src/openfermion/utils/_sparse_tools_test.py
index e01c0c3..02073f9 100644
--- a/src/openfermion/utils/_sparse_tools_test.py
+++ b/src/openfermion/utils/_sparse_tools_test.py
@@ -162,7 +162,7 @@ class JordanWignerSparseTest(unittest.TestCase):
             expected.A))
 
     def test_jw_sparse_1annihilate(self):
-        expected = csc_matrix(([1, 1], ([0, 2], [1, 3])), shape=(4, 4))
+        expected = csc_matrix(([1, -1], ([0, 2], [1, 3])), shape=(4, 4))
         self.assertTrue(numpy.allclose(
             jordan_wigner_sparse(FermionOperator('1')).A,
             expected.A))