Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cirq-google/cirq_google/engine/engine_program.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ async def get_circuit_async(self) -> cirq.Circuit:
Returns:
The program's cirq Circuit.
"""
if self._program is None or self._program.code is None:
# The code field is an any_pb2.Any and is always set. But if the program has not
# been fetched this field may be empty, which we can see by checking the type_url.
if self._program is None or not self._program.code or not self._program.code.type_url:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should it be at the second alternative or not self._program.code.value?

The bool(any_pb2.Any()) is True so the middle expression is effectively or False or ...

Also can we add a test that would cover the fixed failure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is mostly defense in depth because I didn't want to rely on the code attribute being set. As for whether we check type_url or value I think either is probably ok. I don't know if valid proto messages can ever have an empty byte representation, but I presume not? Do you have a particular reason to prefer value over type_url?

Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas Jan 22, 2026

Choose a reason for hiding this comment

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

If we trust the type annotation self._program can be either None or a QuantumProgram which is guaranteed to have a .code field of the any_pb2.Any type.

I was puzzled because or not self._program.code is effectively or False and thus redundant. I wondered if the intent was to have some non-const expression there so I suggested the value field. Otherwise I have no preference, dropping or not self._program.code would be fine with me too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's the types of QuantumProgram itself that concerns me, because the proto-plus library does some metaclass magic so it's not clear to me how its type annotations work. I'd prefer to keep this for defense in depth for now.

self._program = await self.context.client.get_program_async(
self.project_id, self.program_id, True
)
Expand Down
6 changes: 4 additions & 2 deletions cirq-google/cirq_google/engine/engine_program_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,14 @@ def test_get_circuit_v1(get_program_async):


@mock.patch('cirq_google.engine.engine_client.EngineClient.get_program_async')
def test_get_circuit_v2(get_program_async):
@pytest.mark.parametrize("include_empty_program", [False, True])
def test_get_circuit_v2(get_program_async, include_empty_program: bool) -> None:
circuit = cirq.Circuit(
cirq.X(cirq.GridQubit(5, 2)) ** 0.5, cirq.measure(cirq.GridQubit(5, 2), key='result')
)

program = cg.EngineProgram('a', 'b', EngineContext())
program_msg = quantum.QuantumProgram() if include_empty_program else None
program = cg.EngineProgram('a', 'b', EngineContext(), _program=program_msg)
get_program_async.return_value = quantum.QuantumProgram(code=_PROGRAM_V2)
cirq.testing.assert_circuits_with_terminal_measurements_are_equivalent(
program.get_circuit(), circuit
Expand Down
Loading