Skip to content
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

Added qubit partition functions #540

Merged
merged 9 commits into from Nov 5, 2019

Conversation

@obriente
Copy link
Contributor

obriente commented Oct 11, 2019

An implementation of the partial state tomography methods for qubits that we introduced in https://arxiv.org/pdf/1908.05628.pdf .

@googlebot googlebot added the cla: yes label Oct 11, 2019
@babbush babbush requested a review from kevinsung Oct 11, 2019
obriente added 2 commits Oct 11, 2019
def binary_partition_iterator(qubit_list, num_partitions=None):
'''Generator for a list of 2-partitions of N qubits
such that all pairs of qubits are split in at least one partition,
following ArXiv:1908.05628

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

The code in this function does not follow the paper as far as I can tell; the scheme in the paper is based on using the binary representations of the qubit indices. Is there a reason you don't follow the paper? It's difficult for me to verify the correctness of the algorithm you give here.

This comment has been minimized.

Copy link
@obriente

obriente Oct 15, 2019

Author Contributor

Sorry for not detailing the difference - I coded it this way because the output is more balanced, and because this zipping method seems faster/more pythonic than trying to deal with binary division in python.

The code does follow the paper in the case where len(qubit_list) = 2^n. By cutting the list in two and zipping it back together we in effect shuffle the indices.

For example, consider the list [1, 2, 3, 4, 5, 6, 7, 8]. Both the code and the paper first iterate to generate [[1, 2, 3, 4], [5, 6, 7, 8]].
Then, in the paper, we should take the second index, i.e. [[1, 2] + [5, 6], [3, 4] + [7, 8]].
In the code, we zip the list together to generate [1, 5, 2, 6, 3, 7, 4, 8], and then split this again in 2 to obtain [[1, 5, 2, 6], [3, 7, 4, 8]]. As we don't care about order in the lists, the two sets are equivalent.

When len(qubit_list) != 2^n, the code and the paper differ, but the result of the code is better as it is more balanced. For example, consider the list [1, 2, 3, 4, 5] - the paper would want to split this initially into [[1, 2, 3, 4], [5]], while the code would split this into [[1, 2, 3], [4, 5]]. The former would be less preferable in an experiment as we wind up making more repeat measurements of the form XX/YY/ZZ on pairs of qubits.

The new code also has the nice effect for k>2 that the partitions are ordered 'correctly' for the sub-partitioning in Fig.1(b) of the paper. For example, if I look again at [1,2,3,4,5,6,7,8], after partitioning into [[1, 2] + [5, 6], [3, 4] + [7, 8]], the scheme in the paper requires me to partition [1, 2, 5, 6] into [1] + [5], [2] + [6]. In the code, as our list is ordered already as [1, 5, 2, 6], we can repeat the direct division into half.

I've updated the tests to check that the code behaves appropriately for k=3 and N=1 to 15 (previously it only checked for k=3, N=8) - this is the 'test_partition_three' function in the test suite. This checks that for each index [i, j, k] we find one partition that exactly divides them, as required. I've also added some comments for clarity. Hope this helps.

'''

# Some edge cases
if num_iterations is not None and num_iterations == 0:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Just

if num_iterations == 0:

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Thanks, fixed

if num_qubits < 2:
raise ValueError('Need at least 2 qubits to partition')
if num_qubits == 2:
yield [[qubit_list[0]], [qubit_list[1]]]

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Yield tuples instead of lists. So just

yield [qubit_list[0]], [qubit_list[1]]

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Done

for j in range(num_iterations):
# Divide the qubit list in two and return it
partition = [qubit_list[:half_point],
qubit_list[half_point:]]

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Yield a tuple instead of a list.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Sounds good, I've done this throughout the three functions in the file and added some checks in the test suite to ensure that they are indeed returning tuples.

Args:
qubit_list(list): list of qubits to be partitioned
partition_size(int): the number of partitions.

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

"the number of sets in a partition"

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Fixed, thanks



def binary_partition_iterator(qubit_list, num_iterations=None):
'''Generator for a list of 2-partitions of N qubits

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Use double quotes instead of single quotes.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Fixed



num_iterations = num_iterations or\
int(numpy.ceil(numpy.log2(num_qubits)))

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Checking for the None case looks better than doing a line continuation:

if num_iterations is None:
    num_iterations = ...

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Changed

# First iterate over the outer binary partition
outer_iterator = binary_partition_iterator(
qubit_list, num_iterations=num_iterations)
for p1, p2 in outer_iterator:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

I think you chose the letter p for partition, but these are really sets in a partition. So I suggest renaming to set1 and set2.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Good idea, done


# Iterate over all possibilities of subdividing the first
# partition into l partitions and the second partition into
# k - l partitions.

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

I think you're mixing up the word "partition" with "set in partition". Perhaps you mean

        # Iterate over all possibilities of partitioning the first
        # set into l parts and the second set into k - l parts.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Yes, indeed, thanks for resolving the confusion

# subdivide the first partition
inner_iterator1 = partition_iterator(
p1, inner_partition_size, num_iterations)
for ips1 in inner_iterator1:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Better variable name for ips1.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

I've changed it to 'inner_partition1'

inner_iterator2 = partition_iterator(
p2, partition_size-inner_partition_size,
num_iterations)
for ips2 in inner_iterator2:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Better variable name for ips2.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

I've changed it to 'inner_partition2'

print('Testing {}, {}, {}'.format(i, j, k))
pi = partition_iterator(qubit_list, 3)
count = 0
for p1, p2, p3 in pi:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

Check that p1, p2, and p3 are a valid partition, i.e., that their disjoint union is the same set as qubit_list.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Good idea, thanks for the suggestion

for p1, p2, p3 in pi:
print('Partition obtained: ', p1, p2, p3)
if max([sum([1 for x in p if x in [i, j, k]])
for p in [p1, p2, p3]]) == 1:

This comment has been minimized.

Copy link
@kevinsung

kevinsung Oct 15, 2019

Collaborator

The brackets [ are unnecessary in the arguments to max and sum.

This comment has been minimized.

Copy link
@obriente

obriente Oct 16, 2019

Author Contributor

Thanks, I didn't know that

@kevinsung

This comment has been minimized.

Copy link
Collaborator

kevinsung commented Nov 4, 2019

Hi @obriente , I think you missed a few of my comments. Perhaps you need to expand the "hidden conversations" to see them?

@obriente

This comment has been minimized.

Copy link
Contributor Author

obriente commented Nov 5, 2019

Indeed I did - sorry about that. I think I've got all of them now.

@kevinsung kevinsung merged commit 0bc1390 into quantumlib:master Nov 5, 2019
3 checks passed
3 checks passed
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 99.639%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.