New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hamiltonian split by stabilizers #545
Conversation
raise TypeError('Input stabilizers must be QubitOperator or list.') | ||
|
||
stabilizer_list = list(copy.deepcopy(stabilizers)) | ||
ham = copy.deepcopy(hamiltonian) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it necessary to copy these two objects? It doesn't appear to me that you modify them anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Kevin,
Thanks for reviewing the PR, sorry that it took a while for me to update it.
Indeed, these objects are never modified, and hence there is no need to deepcopy them.
I'm going to remove the additional variable name as it is also redundant, but I'm happy to put it back.
|
||
def _check_stabilizer_overlap(pauli_op, stabilizer): | ||
""" | ||
Auxiliar function of get Hamiltonian subsets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Auxiliary"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected! Sorry about this.
overlap = False | ||
if qp[0] in qbts_in_stab: | ||
overlap = True | ||
return overlap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can rewrite this function as
indices_in_pauli_op = {index for (index, _) in list(pauli_op.terms.keys())[0]}
indices_in_stabilizer = {index for (index, _) in list(stabilizer.terms.keys())[0]}
return not indices_in_pauli_op.isdisjoint(indices_in_stabilizer_op)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this piece of code, very simple and does the job. Changed!
|
||
def _check_missing_paulis(hamiltonian, subsets): | ||
""" | ||
Auxiliar function of Hamiltonian subset splitting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Auxiliary"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected! Sorry about this.
from openfermion.ops import QubitOperator | ||
|
||
|
||
def _check_stabilizer_overlap(pauli_op, stabilizer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't really have anything to do with stabilizers. So rename it to _has_overlapping_indices(op1, op2)
, and remove references to stabilizers in the docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, the functions only aims to check if two Pauli operators have common qubits.
Updated name and docstring.
op2(QubitOperator): Single Pauli string. | ||
|
||
Returns: | ||
overlap (Boolean): True if the operator and stabilizer overlap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated. "True if the operators have disjoint indices, False otherwise."
""" | ||
Auxiliary function of get Hamiltonian subsets. | ||
|
||
Check if two QubitOperators have any qubit in common. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated. I think a one-liner "Check if two operators have disjoint indices." is sufficient. Put it on the same line as the triple quotes.
# Initialize subset | ||
ham_subset = QubitOperator() | ||
for pauli in hamiltonian: | ||
if _has_disjoint_indices(pauli, stab) is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is True
is unnecessary; remove it.
try: | ||
remaining_paulis.terms.pop(list(pauli.terms.keys())[0]) | ||
except: | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the try-except necessary? Isn't the Pauli here guaranteed to be inside `remaining_paulis'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xabomon what's the status of this?
return indices_in_op1.isdisjoint(indices_in_op2) | ||
|
||
|
||
def get_hamiltonian_subsets(hamiltonian, stabilizers): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These needs a more descriptive name
Decided to close PR in last OF sync. Functional will be marginally useful. |
Hi all,
I split the PR that I submitted a couple of weeks ago.
Here I add a module that allows one to find the subsets of Pauli operators of a Hamiltonian that have support on a set of stabilizers.
The code comes fully documented and tested as requested by OpenFermion guidelines.
Best,
Xavi