Skip to content

Conversation

Lucaman99
Copy link
Contributor

This pull request adds an implementation of Simon's Algorithm to the examples folder. There is still a bug that occasionally occurs when the code is run, where the secret function is not constructed properly and the simulation fails. I'm looking into the cause of this issue right now, but I thought I would just submit a pull request now, so the code-review process can start as soon as possible.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@vtomole
Copy link
Collaborator

vtomole commented Aug 2, 2019

@Lucaman99 There is no code in this pull request.

@Lucaman99
Copy link
Contributor Author

Sorry about that, I was editing the wrong file on my local branch

Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start! Move everything into a function and add

if __name__ == '__main__':
    main()

at the end of the file. Also, make sure all the builds pass: https://github.com/quantumlib/Cirq/pull/1908/checks?check_run_id=184024079

@@ -0,0 +1,339 @@
'''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'''
'''Demonstrates Simon's algorithm.

@@ -0,0 +1,339 @@
'''
---------------------------------
SIMONS'S ALGORITHM - OVERVIEW
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SIMONS'S ALGORITHM - OVERVIEW

SIMONS'S ALGORITHM - OVERVIEW
---------------------------------
Simon's Algorithm solves the problem of finding a particular value of s when some function f:{0, 1}^n --> {0, 1}^n
is inputted into the program that follows this rule: "f(x) = f(y) if and only if x (+) y is in the set {0^n, s}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is inputted into the program that follows this rule: "f(x) = f(y) if and only if x (+) y is in the set {0^n, s}"
is inputted into the program that follows this rule: "f(x) = f(y) if and only if x xor y is in the set {0^n, s}"

---------------------------------
Simon's Algorithm solves the problem of finding a particular value of s when some function f:{0, 1}^n --> {0, 1}^n
is inputted into the program that follows this rule: "f(x) = f(y) if and only if x (+) y is in the set {0^n, s}"
---------------------------------
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this STEPS section and replace it with a reference to Simon's Paper: https://epubs.siam.org/doi/10.1137/S0097539796298637


# Qubit preparation

number_qubits = #Number of qubits
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this variable set to?


last_work = copy.deepcopy(list(element))

#Check for linear independence
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#Check for linear independence
# Check for linear independence

simulator = cirq.Simulator()
result = simulator.run(c, repetitions=number_qubits-1)
final = result.histogram(key='y')
print("Secret String: "+str(s))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use format() when printing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or f strings f"like this {variable_name}"

return last_work

def construct_solve(matrix_out):

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining what this procedure does.

processing2 = ''.join(str(x) for x in try_2)

the_last = 0
if (secret_function[1][secret_function[0].index(0)] == secret_function[1][secret_function[0].index(int(processing1, 2))] and secret_function[1][secret_function[0].index(0)] == secret_function[1][secret_function[0].index(int(processing2, 2))]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is too long. Create an appropriately named and commented procedure that does this.

r = run[3]

def shuffle_op(matrix, point):

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comments in what this does.

@vtomole
Copy link
Collaborator

vtomole commented Sep 25, 2019

@Lucaman99 You don't have to make the change yourself for one liners if they are suggested. Simply click "commit suggestion".


# Qubit preparation

number_qubits = #Number of qubits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment. Don't repeat what the code says word for word.

Copy link
Contributor

@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that with some effort we can get these three hundred lines down to fifty much clearer lines. But there's a lot of refactoring between here and there. I've left a lot of individual comments to get you started.

#Hard-code oracle on a case-by-case basis


for o in range(0, len(secret_function[0])):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid o since it looks like zero.


for o in range(0, len(secret_function[0])):
counter = 0
for j in list(str(format(secret_function[0][o], "0"+str(number_qubits)+"b"))):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strikes me as "stringly typed programming", where you are unnecessarily encoding the logic into string operations.

s = domain[random.randint(0, len(domain)-1)]

#Create the "secret function"
for g in range(0, int((2**number_qubits)/2)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract this entire section about making the secret function into a method called create_secret_function

co_domain = []
fixed = []

for k in range(0, 2**number_qubits):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

domain = list(range(2**number_qubits))
selector = list(domain)
fixed = list(domain)
co_domain = [False] * 2**number_qubits


secret_function = [fixed, co_domain]

oracle = make_oracle(ancilla, secret_function, first_qubits, second_qubits, s, the_activator)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use better names than first_qubits and second_qubits. Refer to their role in the algorithm.

Rename s to a word.

Drop the the in the_activator.

for ik in range (0, len(hol)):
if (hol[ik] == hol[ij] and ik != ij):
ha = False
if (ha == True and calc == [0 for p in range(0, len(calc))]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ha and calc == [0] * len(calc):

for i in range(1, len(matrix)+1):
for c in list(itertools.combinations([y[0:len(matrix)+1] for y in matrix], i)):
hol = []
for b in c:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hol = list(c)


for i in range(0, len(matrix)):
for j in range(i+1, len(matrix)):
if (matrix[i][i] != 0):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this condition into the outer loop

for i in range(0, len(matrix)):
for j in range(i+1, len(matrix)):
if (matrix[i][i] != 0):
x = -1*matrix[j][i]/matrix[i][i]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this block is just matrix[j] -= matrix[i] * int(matrix[j][i] / matrix[i][i]), assuming matrix is a numpy array.


for j in range(0, len(work[0])-1):
temporary = []
for g in range(0, len(work)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary = [e for e in work if e[j] == 1]

@Strilanc
Copy link
Contributor

I think if you add all the relevant email addresses to your github account it might resolve the issue. You do need to sign the CLA from each of them.

For now I wouldn't worry about the CLA; there's lots of refactoring to do first.

@Lucaman99
Copy link
Contributor Author

@Strilanc Yeah, we're talking about the CLA in relation to my other pull request #1934, which seems to be ready to be merged.

@vtomole
Copy link
Collaborator

vtomole commented Sep 25, 2019

@Lucaman99 Far from ready. I just like seeing green check-marks on pull requests :D. I want to leave the reviews in rounds so it's easy to respond in chunks.

@vtomole
Copy link
Collaborator

vtomole commented Dec 11, 2019

Hey @Lucaman99, how's this going?

@vtomole
Copy link
Collaborator

vtomole commented Feb 18, 2020

Closing as there is another Simon's Pull request open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants