Skip to content

Add channel random circuit test coverage#1080

Closed
arnavnagzirkar wants to merge 3 commits into
tensorflow:masterfrom
arnavnagzirkar:fix-1066-clean
Closed

Add channel random circuit test coverage#1080
arnavnagzirkar wants to merge 3 commits into
tensorflow:masterfrom
arnavnagzirkar:fix-1066-clean

Conversation

@arnavnagzirkar

@arnavnagzirkar arnavnagzirkar commented Jun 3, 2026

Copy link
Copy Markdown

Adds regression coverage for random circuit generation with channels.

The include_channels=True argument mentioned here is a parameter of two functions in the tensorflow_quantum.python.util module (imported as util in the tests):

  • util.random_circuit_resolver_batch
  • util.random_symbol_circuit_resolver_batch

The new tests in tensorflow_quantum/python/util_test.py assert that, when each of these functions is called with include_channels=True, the generated random circuits contain at least one supported channel operation (taken from util.get_supported_channels()).

Following the Gemini Code Assist review, the tests also verify that the circuits survive a full serialization round trip: util.convert_to_tensor is used to serialize, util.from_tensor to deserialize, and the re-serialized result is compared for equality. This is stronger than only checking that serialization does not return null.

This addresses issue #1066.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds two new unit tests, test_random_circuit_resolver_batch_channels_present and test_random_symbol_circuit_resolver_batch_channels_present, to verify that channel operations are included in generated circuits when include_channels=True and that the resulting circuits are serializable. The reviewer recommends strengthening the serialization checks in both tests by performing a full round-trip serialization and deserialization verification instead of only asserting that the serialized output is not null.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tensorflow_quantum/python/util_test.py Outdated
Comment thread tensorflow_quantum/python/util_test.py Outdated
Address review feedback: verify channel-containing circuits survive a full convert_to_tensor/from_tensor round-trip instead of only asserting non-null serialization.
@mhucka

mhucka commented Jun 5, 2026

Copy link
Copy Markdown
Member

@arnavnagzirkar Thank you for this contribution. Gemini Code Assist's review comments seem reasonable. If you don't disagree with them, could you got ahead and make those changes?

Also, in the PR description, it talks about include_channels=True, but it doesn't mention which function or module that refers to. Could you add that information to the description?

@mhucka mhucka self-assigned this Jun 5, 2026
@mhucka mhucka added area/health Involves general matters of project configuration, health, maintenance, and similar concerns area/tests Involves tests and testing of the TFQ codebase labels Jun 5, 2026
@mhucka mhucka changed the title test: add channel random circuit coverage Add channel random circuit test coverage Jun 5, 2026
@mhucka mhucka added status/awaiting-response Waiting on a response from someone priority/for-0.7.7 To be done before releasing TFQ version 0.7.7 labels Jun 5, 2026
@arnavnagzirkar

Copy link
Copy Markdown
Author

Thanks @mhucka. Two quick updates:

  1. The Gemini round trip suggestion is already in the branch. I applied it in commit 9b1f9a8, which I pushed before your comment, so it was probably just not obvious that the threads were already handled. Both new tests now serialize with util.convert_to_tensor(..., deterministic_proto_serialize=True), deserialize with util.from_tensor, and assert that re-serializing the deserialized circuits gives back the same tensor. That makes it a full round trip rather than the earlier not-null check.

  2. I updated the PR description to say where include_channels=True comes from. It is a parameter of util.random_circuit_resolver_batch and util.random_symbol_circuit_resolver_batch in the tensorflow_quantum.python.util module.

Let me know if you would like anything else adjusted.

@mhucka

mhucka commented Jun 9, 2026

Copy link
Copy Markdown
Member

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds two new unit tests to tensorflow_quantum/python/util_test.py: test_random_circuit_resolver_batch_channels_present and test_random_symbol_circuit_resolver_batch_channels_present. These tests verify that channel operations are correctly included in generated circuits when include_channels=True and that the resulting circuits can be successfully serialized and deserialized. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@mhucka mhucka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for working on this. I have a small change request, but otherwise this looks good and almost ready to merge.

channel_types = tuple(type(c) for c in util.get_supported_channels())

circuits, _ = util.random_circuit_resolver_batch(qubits,
batch_size=5,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For this and the next new function, could you please use a variable to set the batch size, following the pattern of the other functions in this file? (See for example this line: https://github.com/tensorflow/quantum/pull/1080/changes#diff-0605f6d13dea49e94d3ddf78736d228215528e5f046a281dfddc1ad9afd63cddR119)

@arnavnagzirkar

Copy link
Copy Markdown
Author

Thanks @mhucka for the review and for the time on this.

I see that #1091 was merged in the meantime and already adds comprehensive channel coverage for the random circuit generators, closing issue #1066. Its tests (test_random_circuit_resolver_batch_channel_content, test_random_symbol_circuit_resolver_batch_channel_content, and test_random_symbol_circuit_channel_content) cover everything in this PR and more, including the include_channels=False path and validation that channel ops are not controlled or parameterized.

Since the two tests here are now a subset of what is already on master, I am closing this PR to avoid duplicate coverage. Happy to help elsewhere if there is a follow up you would like. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/health Involves general matters of project configuration, health, maintenance, and similar concerns area/tests Involves tests and testing of the TFQ codebase priority/for-0.7.7 To be done before releasing TFQ version 0.7.7 status/awaiting-response Waiting on a response from someone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants