Conversation
Introduce `merge` method to `ProgramSetQuantumTaskResult` that combines multiple results into one.
|
If this is approved before #1249, we can merge this cleanly into the |
| return binding.to_ir() | ||
|
|
||
|
|
||
| def _count_executables(binding: CircuitBinding | Circuit) -> int: |
There was a problem hiding this comment.
Surprised CircuitBinding doesn't actually have a simple .executables property here.
There was a problem hiding this comment.
Mainly because there isn't really a class that encapsulates a single executable, so such a method would just return a list of tuples
| return num_ps * num_obs | ||
|
|
||
|
|
||
| def _buffer_result( |
There was a problem hiding this comment.
Can we add a description to this function? The other ones are straightforward but this has more complexity and _buffer_result is a bit vague.
| for composite in result.entries: | ||
| for entry in composite.entries: | ||
| if j >= len(map_k): | ||
| raise ValueError( |
There was a problem hiding this comment.
Can this realistically happen? I guess if you have two programsets, where each one has the same total number of different numbers in the constituents?
There was a problem hiding this comment.
Yeah, if you pass in the wrong merge list; this is just defensive, like the if j != len(parent_indices) below.
| return ProgramSetQuantumTaskResult( | ||
| entries=entries, | ||
| task_metadata=ProgramSetTaskMetadata( | ||
| id=";".join(meta.id for meta in metas), # Better way to do this? |
There was a problem hiding this comment.
I wonder if there is a way to denote that this is just synthetic metadata, or an aggregate, and these are just convenience attributes. Unfortunately the best way for that would probably be a new schema, which seems excessive.
There was a problem hiding this comment.
Or if task_metadata could have a union type with Sequence[ProgramSetTaskMetadata]
There was a problem hiding this comment.
I can add a flag that triggers a warning the first time the metadata is accessed.
| def _buffer_result( | ||
| k: int, | ||
| result: ProgramSetQuantumTaskResult, | ||
| map_k: list[int], |
There was a problem hiding this comment.
this is like the composite_entry_map?
There was a problem hiding this comment.
Map to the parent program set index
| ) | ||
| orig_idx = map_k[j] | ||
| binding_idx, ps_idx, obs_idx = executable_indices[orig_idx] | ||
| buffer[orig_idx] = _convert_measured_entry( |
There was a problem hiding this comment.
So map_k is some contiguous list between [0] and the composite entry number of executables, right? j is essentially a subindex, or the rank-2 index. So why is the buffer at along this index when the original buffer is len(total_executables), i.e. unfolded?
There was a problem hiding this comment.
No, map_k's sublist actually starts where the previous sublist ended; it's the index in the parent program set. I'll rename accordingly.
And other minor changes
2 participants
Introduce
mergemethod toProgramSetQuantumTaskResultthat combines multiple results into one.Issue #, if available:
Description of changes:
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.