Skip to content

Conversation

dstrain115
Copy link
Collaborator

  • Adds a new Message in the Program proto for KeyedCircuit which can store multiple circuits indexed by parameters.
  • This also adds a pair of functions in CircuitSerializer called serialize_multi_program and deserialize_multi_program.
  • These can serialize a list of circuits, a map of circuits with string keys, or a function that returns a circuit (with a corresponding sweep object that specifies how to call the function).

- Adds a new Message in the Program proto for KeyedCircuit which
can store multiple circuits indexed by parameters.
- This also adds a pair of functions in CircuitSerializer called
serialize_multi_program and deserialize_multi_program.
- These can serialize a list of circuits, a map of circuits with string
  keys, or a function that returns a circuit (with a corresponding sweep
  object that specifies how to call the function).
@dstrain115 dstrain115 requested review from a team, verult, vtomole and wcourtney as code owners September 30, 2025 20:30
@github-actions github-actions bot added the size: L 250< lines changed <1000 label Sep 30, 2025
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (1ad77cd) to head (0c2c613).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7678    +/-   ##
========================================
  Coverage   99.37%   99.38%            
========================================
  Files        1085     1089     +4     
  Lines       96937    97526   +589     
========================================
+ Hits        96331    96925   +594     
+ Misses        606      601     -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

I wonder if the creation of circuits from sweep parameters and a circuit factory should be moved to a separate function so we could simplify what argument types are accepted in serialize_multi_program. The type signature there could also accept Iterable[AbstractCircuit] and Iterable[tuple[str, AbstractCircuit]] so that serialize_multi_program could take a generator function output.

results.
Raises:
NotImplementedError: If the program is of a type that is supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: supported --> not supported

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eh, somehow it is still there on lines 136 and 139.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops! Fixed.

Comment on lines 125 to 126
| Callable[..., cirq.AbstractCircuit]
| Callable[..., Mapping[str, cirq.AbstractCircuit]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC, the callable arguments are used to generate circuits from the sweep parameters, essentially translating them to either a sequence or mapping of circuits, ie, the other two parameter types. The sweep argument is not serialized at all, it is only used to provide arguments for the callable.

How about factoring the circuit generation to a separate function, say circuits_from_sweep which would return either a sequence or mapping of Circuits?
If memory overhead is of concern, circuits_from_sweep could be a generator function and the multi_program argument here could be adjusted to accept Iterable[AbstractCircuit] | Iterable[tuple[str, AbstractCircuit]].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I separated this out to serialize_circuit_function(). I don't think having circuits_from_sweep is worth it, since that is not intended functionality. You'd just want to serialize them immediately. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks! I just realized that serialize_circuit_function also serializes param_tuple which is not available in serialize_multi_program (ie, they cannot be chained).

raise ValueError(f'Function returned unrecognized type: {type(circuit_or_map)}')
for key, circuit in circuit_tuples:
new_program = msg.keyed_circuits.add()
if key is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit - this should be always True looking at the code above. Maybe remove?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

("", circuit_or_map)
]
elif isinstance(circuit_or_map, Mapping):
circuit_tuples = list(circuit_or_map.items())
Copy link
Collaborator

Choose a reason for hiding this comment

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

circuit_tuples from different sweep points can have duplicate keys, so the Program.keyed_circuits sequence may get non-unique KeyedCircuit.key values as well.

If that is not an issue, perhaps the serialize_multi_program should also accept an iterable of (key, Circuit) pairs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not totally sure I understand the concern. The combination of key and args should always be unique. I am not sure that the key needs to be unique on its own.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see - there is no requirement for unique keys. NVM, the initial comment, it is OK as is.

@dstrain115 dstrain115 requested a review from pavoljuhas October 1, 2025 12:30
constants=msg.constants,
raw_constants=raw_constants,
)
elif isinstance(multi_program, Sequence):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit - Sequence --> Iterable or allow only Sequence[cirq.Circuit] in multi_program annotation. As it is, generator would pass type check, but raise an exception at runtime.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM with two small corrections.

@dstrain115 dstrain115 added this pull request to the merge queue Oct 2, 2025
Merged via the queue into quantumlib:main with commit 913e68b Oct 2, 2025
35 checks passed
@dstrain115 dstrain115 deleted the multi_program_serialization branch October 2, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: L 250< lines changed <1000

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants