Conversation
| alice_choices[2, :, :, :2] = 1 - self.alice_measurements[2, :, :, :2] | ||
| bob_choices[:, 0, :, 0] = self.bob_measurements[:, 0, :, 1] | ||
| bob_choices[:, 0, :, 1] = self.bob_measurements[:, 0, :, 0] | ||
| bob_choices[:, 1:, :, :2] = self.bob_measurements[:, 1:, :, :2] |
There was a problem hiding this comment.
This is a lot of unexplained matrix manipulation. Can we add a line or two to the docstring to explain how Alice and Bob's choices are generated.
There was a problem hiding this comment.
I made a PR to add some comments to the original version (https://github.com/quantumlib/Cirq/pull/7890/changes) that explain this. @alejogoogle can you give me permission to contribute to your fork of ReCirq so I can copy them here?
| first two. | ||
|
|
||
| Returns: | ||
| Alice and Bob's choices in the game. |
There was a problem hiding this comment.
These are returned as two numpy arrays. Can we explain in what format Alice and Bob's choices are returned?
There was a problem hiding this comment.
I made a PR to document this in the original version (https://github.com/quantumlib/Cirq/pull/7890/changes). @alejogoogle can you give me permission to contribute to your fork of ReCirq so I can copy them here?
There was a problem hiding this comment.
Added to the docstring of generate_choices.
| alice_choices[2, :, :, :2] = 1 - self.alice_measurements[2, :, :, :2] | ||
| bob_choices[:, 0, :, 0] = self.bob_measurements[:, 0, :, 1] | ||
| bob_choices[:, 0, :, 1] = self.bob_measurements[:, 0, :, 0] | ||
| bob_choices[:, 1:, :, :2] = self.bob_measurements[:, 1:, :, :2] |
There was a problem hiding this comment.
This looks very similar to the above code. Is there a way we can combine them?
Same with below.
| if np.sum(alice_triad) % 2 == 0 and np.sum(bob_triad) % 2 == 1: | ||
| number_of_matches += 1 | ||
| if alice_triad[col] == bob_triad[row]: | ||
| win_matrix[row, col] += 1 |
There was a problem hiding this comment.
This looks like repeated code from get_multiply_matrix below. Can we call that instead?
|
In addition to addressing Doug's comments about the code, I think also it's important to add a description to the pull request. Here are some points to address:
I realize it's more work to write a description, and I'm sorry for that. Perhaps an AI tool can help with this in some way. |
| def _select_choices_from_second_data(self) -> tuple[np.ndarray, np.ndarray]: | ||
| return self.alice_measurements, self.bob_measurements | ||
|
|
||
| def _generate_choices_from_rules_guess_3rd(self) -> tuple[np.ndarray, np.ndarray]: |
There was a problem hiding this comment.
@alejogoogle can we call this "infer third" instead of "guess third" since there isn't really any guessing happening. The third outcome is dictated by what they measured for the first two + the rules of the game.
FYI, I called the original function _generate_choices_from_rules because they were using the rules of the game to decide what their choices should be after measuring the first two.
4 participants
No description provided.