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.
| 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?
| 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?
| @@ -0,0 +1,749 @@ | |||
| # Copyright 2025 Google | |||
There was a problem hiding this comment.
We should add tests for this file.
|
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. |
3 participants
No description provided.