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

[Quantum Chinese Chess] Add class Jump(QuantumEffect). #164

Merged
merged 14 commits into from
Oct 30, 2023

Conversation

madcpf
Copy link
Collaborator

@madcpf madcpf commented Oct 17, 2023

  • Add Jump(QuantumEffect) to handle the jump cases;
  • Add method unhook() to QuantumWorld, so that a given object could be replaced with a new ancilla with value 0;
  • Add update_board_by_sampling() in QuantumChineseChess to sample and update the classical properties of the pieces;
  • Update Piece.reset() method to set the is_entangled field;
  • Flip the labeling of the board (row <-> num_row - 1 - row) to make it more natural;
  • Add some test util functions.

@madcpf madcpf marked this pull request as draft October 17, 2023 23:20
@madcpf madcpf requested a review from dstrain115 October 18, 2023 21:42
@madcpf madcpf marked this pull request as ready for review October 18, 2023 21:42
@madcpf madcpf changed the title Add class Jump(QuantumEffect). [Quantum Chinese Chess] Add class Jump(QuantumEffect). Oct 23, 2023
self.circuit = self.circuit.transform_qubits(
lambda q: qubit_remapping_dict.get(q, q)
)
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this return the ancilla?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The method works like reset. (Maybe I should rename it to reset()?) So I don't think user should apply more effects on the ancilla. They should move forward with the reset object, which since now has a new reset value.

@@ -305,6 +305,20 @@ def _interpret_result(self, result: Union[int, Iterable[int]]) -> int:
return result_list[0]
return result

def unhook(self, object: QuantumObject) -> None:
"""Replaces all usages of the given object in the circuit with a new ancilla with value=0."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe leave off the value=0 portion or move it to a new sentence to clarify what you mean. The value=0 is the initial state of the ancilla but it won't have value=0 after all the operations act on it.

Since this is part of the alpha interface, we may want to further explain what this does in more detail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Doug. It's updated. Is it clearer now?

@@ -74,15 +74,11 @@ def from_fen(cls, fen: str = _INITIAL_FEN) -> "Board":
name, SquareState.OCCUPIED, piece_type, color
)
col += 1
row_index -= 1
row_index += 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Next time, please separate this into two PRs, since the print change is independent of the board display change. It's fine to keep it here this time though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Doug. I'll send separate changes with concentric topic next time.

@@ -381,6 +386,9 @@ def apply_move(self, str_to_parse: str) -> None:
quantum_pieces_1,
)

print(move_type, " ", move_variant)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a debug statement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it's now changed to only print above certain debug level.

# i.e. pawn would move only if the target is there, i.e. CNOT(t, s), and an entanglement could be
# created. This could be a general game setting, i.e. we could allow players to choose if they
# want the source piece to move (in case of capture) if the target piece is not there.
source_0, target_0 = objects
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth putting a check that the len(objects) is 2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Doug. I think it's already been checked since we override num_objects()?

source_0, target_0 = objects
world = source_0.world
if self.move_variant == MoveVariant.CAPTURE:
# We peek and force measure source_0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, peek -> pop.

Copy link
Collaborator Author

@madcpf madcpf Oct 25, 2023

Choose a reason for hiding this comment

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

Thanks Doug. I think pop() includes peek and force measurement?

target_0.reset()
elif self.move_variant == MoveVariant.CLASSICAL:
if target_0.type_ != Type.EMPTY:
# For classical moves with target_0 occupied, we replace the qubit of target_0 with
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might be correct here, but why do we need a new ancilla if it is a classical move?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Doug. By "world.unhook(target_0)" I'm trying to reset target_0 to the initial value (i.e. 0), before doing the PhasedMove below. You are right that we don't need an ancilla in this case, I'm changing it to flip(target_0) instead.

assert_fifty_fifty(board_probabilities, locations_to_bitboard(["a2", "b1"]))
assert_fifty_fifty(board_probabilities, locations_to_bitboard(["a3", "b1"]))
Jump(MoveVariant.CAPTURE)(world["a2"], world["b1"])
# pop() will break the supersition and only one of the following two states are possible.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:superposition

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. Thanks.

else:
assert_samples_in(board, [locations_to_bitboard(["a3", "b1"])])

# Target is in quantum state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider splitting this to a new test. (and below too)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Doug. Updated into separate tests.

unitary/examples/quantum_chinese_chess/test_utils.py Outdated Show resolved Hide resolved
Pengfei Chen added 2 commits October 25, 2023 12:48
@madcpf madcpf requested a review from dstrain115 October 25, 2023 19:50
@madcpf madcpf merged commit ab75eb3 into quantumlib:main Oct 30, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants