From f4cfdec34da076696870843a66db777ba89202a3 Mon Sep 17 00:00:00 2001 From: stark256-spec Date: Wed, 12 Aug 2026 01:03:06 -0500 Subject: [PATCH 1/2] Fix #880: return simplified QubitOperator from symmetry_conserving_bravyi_kitaev symmetry_conserving_bravyi_kitaev could return QubitOperators with un-simplified terms: remove_indices() shifts qubit indices and can map two qubits onto the same index, producing a term with multiple Paulis acting on one qubit (e.g. ((0, 'X'), (1, 'Y'), (1, 'X'))). Because those terms are written straight into the operator's .terms dict, they bypass the simplification normally done on construction. get_sparse_operator (via qubit_operator_sparse) assumes each qubit appears at most once per term, so it raised 'ValueError: axis 0 index 7 exceeds matrix dimension 4' on such operators. Rebuild the operator after remove_indices so every term is routed back through QubitOperator's simplification, restoring canonical one-Pauli-per-qubit form. Add a regression test using the reporter's reproducer that checks the terms are canonical and that the resulting sparse operator matches the explicitly-simplified operator. Closes #880 --- .../opconversions/remove_symmetry_qubits.py | 14 ++++++++-- .../remove_symmetry_qubits_test.py | 27 ++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py b/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py index 5356d8d54..41ec02120 100644 --- a/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py +++ b/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py @@ -16,7 +16,7 @@ import copy -from openfermion.ops.operators import FermionOperator +from openfermion.ops.operators import FermionOperator, QubitOperator from openfermion.transforms.opconversions.bravyi_kitaev_tree import bravyi_kitaev_tree from openfermion.transforms.opconversions.term_reordering import reorder from openfermion.utils.indexing import up_then_down @@ -99,7 +99,17 @@ def symmetry_conserving_bravyi_kitaev(fermion_hamiltonian, active_orbitals, acti ) qubit_hamiltonian = remove_indices(qubit_hamiltonian, (active_orbitals / 2, active_orbitals)) - return qubit_hamiltonian + # remove_indices() shifts qubit indices and can map two qubits onto the + # same index, producing terms with multiple Paulis acting on one qubit + # (e.g. ((0, 'X'), (1, 'Y'), (1, 'X'))). Rebuilding the operator routes + # each term back through QubitOperator's simplification, restoring the + # canonical one-Pauli-per-qubit form that consumers such as + # get_sparse_operator require. See issue #880. + simplified_hamiltonian = QubitOperator() + for term, coefficient in qubit_hamiltonian.terms.items(): + simplified_hamiltonian += QubitOperator(term, coefficient) + + return simplified_hamiltonian def edit_hamiltonian_for_spin(qubit_hamiltonian, spin_orbital, orbital_parity): diff --git a/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py b/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py index 1a95e48f3..0f59d497c 100644 --- a/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py +++ b/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py @@ -16,6 +16,7 @@ import unittest +import numpy import pytest from openfermion.hamiltonians import fermi_hubbard @@ -26,7 +27,7 @@ jw_get_ground_state_at_particle_number, ) from openfermion.linalg import eigenspectrum -from openfermion.ops.operators import FermionOperator +from openfermion.ops.operators import FermionOperator, QubitOperator from openfermion.transforms.opconversions.remove_symmetry_qubits import ( symmetry_conserving_bravyi_kitaev, @@ -152,3 +153,27 @@ def test_single_operator(self): e_trafo = eigenspectrum(trafo_op) # Check eigenvalues self.assertSequenceEqual(e_op.tolist(), e_trafo.tolist()) + + def test_output_is_simplified_qubit_operator(self): + # Regression test for issue #880: symmetry_conserving_bravyi_kitaev + # used to return QubitOperators with un-simplified terms (multiple + # Paulis acting on the same qubit), which made get_sparse_operator + # raise a ValueError. + op = FermionOperator("0^ 1^") + trafo_op = symmetry_conserving_bravyi_kitaev(op, active_orbitals=4, active_fermions=2) + + # Every term must be in canonical form: at most one Pauli per qubit. + for term in trafo_op.terms: + qubits = [qubit for qubit, _ in term] + self.assertEqual(len(qubits), len(set(qubits))) + + # get_sparse_operator must no longer raise ... + sparse_op = get_sparse_operator(trafo_op) + + # ... and must match the explicitly-simplified operator. + simplified = QubitOperator() + for term, coefficient in trafo_op.terms.items(): + simplified += QubitOperator(term, coefficient) + expected = get_sparse_operator(simplified) + + self.assertTrue(numpy.allclose(sparse_op.toarray(), expected.toarray())) From a585261821dd2406dd574b430a83d2faf959e9fd Mon Sep 17 00:00:00 2001 From: stark256-spec Date: Thu, 13 Aug 2026 15:21:26 -0500 Subject: [PATCH 2/2] Address review: use integer division and a non-circular regression test - Use // for the active_orbitals/2 index computations passed to edit_hamiltonian_for_spin and remove_indices (integer indices). - The regression test previously built its 'expected' operator by re-simplifying the (already simplified) output, so it compared the result against itself. Compare against an independently specified, hand-verified QubitOperator instead. --- .../opconversions/remove_symmetry_qubits.py | 4 +-- .../remove_symmetry_qubits_test.py | 35 ++++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py b/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py index 41ec02120..9ff91361c 100644 --- a/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py +++ b/src/openfermion/transforms/opconversions/remove_symmetry_qubits.py @@ -95,9 +95,9 @@ def symmetry_conserving_bravyi_kitaev(fermion_hamiltonian, active_orbitals, acti qubit_hamiltonian, active_orbitals, parity_final_orb ) qubit_hamiltonian = edit_hamiltonian_for_spin( - qubit_hamiltonian, active_orbitals / 2, parity_middle_orb + qubit_hamiltonian, active_orbitals // 2, parity_middle_orb ) - qubit_hamiltonian = remove_indices(qubit_hamiltonian, (active_orbitals / 2, active_orbitals)) + qubit_hamiltonian = remove_indices(qubit_hamiltonian, (active_orbitals // 2, active_orbitals)) # remove_indices() shifts qubit indices and can map two qubits onto the # same index, producing terms with multiple Paulis acting on one qubit diff --git a/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py b/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py index 0f59d497c..21257d561 100644 --- a/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py +++ b/src/openfermion/transforms/opconversions/remove_symmetry_qubits_test.py @@ -157,23 +157,32 @@ def test_single_operator(self): def test_output_is_simplified_qubit_operator(self): # Regression test for issue #880: symmetry_conserving_bravyi_kitaev # used to return QubitOperators with un-simplified terms (multiple - # Paulis acting on the same qubit), which made get_sparse_operator - # raise a ValueError. + # Paulis acting on the same qubit, e.g. ((0, 'X'), (1, 'Y'), (1, 'X')) + # left behind when remove_indices maps two qubit indices onto one), + # which made get_sparse_operator raise a ValueError. op = FermionOperator("0^ 1^") trafo_op = symmetry_conserving_bravyi_kitaev(op, active_orbitals=4, active_fermions=2) - # Every term must be in canonical form: at most one Pauli per qubit. + # The result must equal the known, fully-simplified operator for this + # input, worked out independently (rather than re-derived from + # trafo_op, which would be a no-op now that the output is simplified). + expected_op = ( + QubitOperator(((0, "X"), (1, "Z")), -0.25) + + QubitOperator(((0, "X"),), -0.25) + + QubitOperator(((0, "Y"), (1, "Z")), 0.25j) + + QubitOperator(((0, "Y"),), 0.25j) + ) + self.assertEqual(trafo_op, expected_op) + + # Every term is canonical: at most one Pauli per qubit. for term in trafo_op.terms: qubits = [qubit for qubit, _ in term] self.assertEqual(len(qubits), len(set(qubits))) - # get_sparse_operator must no longer raise ... - sparse_op = get_sparse_operator(trafo_op) - - # ... and must match the explicitly-simplified operator. - simplified = QubitOperator() - for term, coefficient in trafo_op.terms.items(): - simplified += QubitOperator(term, coefficient) - expected = get_sparse_operator(simplified) - - self.assertTrue(numpy.allclose(sparse_op.toarray(), expected.toarray())) + # get_sparse_operator must no longer raise on the result, and must + # produce the sparse matrix of the independently-specified operator. + self.assertTrue( + numpy.allclose( + get_sparse_operator(trafo_op).toarray(), get_sparse_operator(expected_op).toarray() + ) + )