[Quantum Chinese Chess] Implement more move types #172
Conversation
| @@ -402,7 +402,18 @@ def apply_move(self, str_to_parse: str) -> None: | |||
| Jump(move_variant)(source_0, target_0) | |||
| elif move_type == MoveType.JUMP: | |||
| Jump(move_variant)(source_0, target_0) | |||
| # TODO(): apply other move types. | |||
| elif move_type == MoveType.SLIDE: | |||
There was a problem hiding this comment.
what version of python is this targetting? With 3.11 this big if/else can be made much nicer with:
match move_type:
case MoveType.CLASSICAL:
# code
case MoveType.JUMP:
# codeI think we'd also get lint checks of not all enum values are checked.
There was a problem hiding this comment.
The dependency is python3.9. Maybe we can change it later when 3.11 is the dependency. Adding a TODO() here. Thanks.
| def num_objects(self) -> Optional[int]: | ||
| return 2 | ||
|
|
||
| def effect(self, *objects) -> Iterator[cirq.Operation]: |
There was a problem hiding this comment.
Is there any expectation of type of values in *objects? perhaps add typing annotation?
There was a problem hiding this comment.
This method is derived from QuantumEffect. I've updated the methods in QuantumEffects with typing annotation. Thanks.
| return 2 | ||
|
|
||
| def effect(self, *objects) -> Iterator[cirq.Operation]: | ||
| source_0, target_0 = objects |
There was a problem hiding this comment.
this might be error-prone, the method signature says any number of objects, but this unpacking will break if not exactly two.
There was a problem hiding this comment.
Yeah, the function signature is from the super class.
However, at the very least, we probably want a better error message if the wrong number of objects are passed in.
There was a problem hiding this comment.
This is a subclass of QuantumEffect, which will run a _verify_objects() method that will verify num_dimension() and num_objects() are satisfied by the objects. Thanks.
There was a problem hiding this comment.
Thanks Doug. QuantumEffect._verify_objects() returns the following error message now.
def _verify_objects(self, *objects: "QuantumObject"):
if self.num_objects() is not None and len(objects) != self.num_objects():
raise ValueError(f"Cannot apply effect to {len(objects)} qubits.")
...
| # TODO(): consider specific cases to save the ancilla. | ||
| # Add a new ancilla to indicate whether the fire could be made (value = 1 means it could). | ||
| capture_ancilla = world._add_ancilla( | ||
| f"{expect_occupied_path_piece.name}" |
There was a problem hiding this comment.
is expect_occupied_path_piece.name not guaranteed to be a string?
There was a problem hiding this comment.
Yes it is a string. It's fixed now. Thanks.
| @@ -21,7 +21,7 @@ | |||
| MoveType, | |||
| MoveVariant, | |||
| ) | |||
| from unitary.examples.quantum_chinese_chess.move import Jump | |||
| from unitary.examples.quantum_chinese_chess.move import * | |||
There was a problem hiding this comment.
Importing star is not recommended. In fact, the style guide recommends not even importing individual classes:
go/pystyle#imports
There was a problem hiding this comment.
Thanks Doug. I'm changing away from *. Maybe later I can changing away from importing individual classes across all files.
| return 2 | ||
|
|
||
| def effect(self, *objects) -> Iterator[cirq.Operation]: | ||
| source_0, target_0 = objects |
There was a problem hiding this comment.
Yeah, the function signature is from the super class.
However, at the very least, we probably want a better error message if the wrong number of objects are passed in.
Implement the following types of moves: