Conversation
`ProgramSet.split` takes in a `max_excutables` parameter, so that the generated program sets will contain at most that many executables. Also fixed coverage output when running `tox`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1249 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 169 169
Lines 10963 11061 +98
Branches 1412 1433 +21
=========================================
+ Hits 10963 11061 +98 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sesmart
left a comment
sesmart
left a comment
There was a problem hiding this comment.
Very cool 😄, just some notes on usability. Any reason the input_set is taken as the major counting index before observables? Just coincidence of orderings?
speller26
changed the title
Will move to separate PR
speller26
changed the title
|
Results merging has been moved to #1254 |
| raise ValueError("No per-executable shots defined") | ||
| return self._shots_per_executable * self.total_executables | ||
|
|
||
| def enumerate_executables(self) -> Iterator[tuple[int, int, int]]: |
There was a problem hiding this comment.
Hm...should this be a property? Like enumeration?
| def split(self, max_executables: int) -> tuple[list[ProgramSet], list[list[int]]]: | ||
| """Split this program set into program sets of at most ``max_executables`` executables, | ||
| alongside a map that records the position in the original program set of each executable | ||
| in each of the generated program sets. |
There was a problem hiding this comment.
would add, i.e. indexed from (0, total_executables-1)
| ``max_executables`` due to its observable list or ``Sum`` Hamiltonian being larger than | ||
| the budget, the observable list is split into chunks of at most ``max_executables`` entries | ||
| (``Sum`` summands are sliced with coefficients preserved). Observable splitting is only | ||
| performed when necessary; otherwise the full observable list or ``Sum`` is kept intact. |
There was a problem hiding this comment.
Does this have consequences for max splitting? What is the worst case example here? I.e. CB of only observables that are at the border of being too big for 2 to fit in one? I.e. 10 executables but only a 19 ps split? This approaches ~0.5 filling efficiency for large N.
I think a flatten option might be nice, just so the user knows it has been split 1-1, though it adds complexity to the merge as well. Is there a simpler way to approach/think about this?
There was a problem hiding this comment.
Also, if it doesn't, that is okay - maybe we can just make it a bit more clear.
There was a problem hiding this comment.
It also seems to occur on the other side of the approach - 10 executables, but only a 9 split limit causes -> 0.5 filling efficiency.
There was a problem hiding this comment.
It also just occured to me we kind of have a flatten...bind_observables_to_inputs would do something like this - but there is no, "unbind", or reorder. It would be cool if merge could take that as an input as well though, as it should also have the same ordering.
|
|
||
| return program_sets, index_map | ||
|
|
||
| def _executable_blocks(self, max_executables: int) -> list[_ExecutableBlock]: |
There was a problem hiding this comment.
Sorry, this definitely needs a description 😢
There was a problem hiding this comment.
Rather, maybe a detailed description of the splitting approach.
| current = [] | ||
| current_size = 0 | ||
| for block in self._executable_blocks(max_executables): | ||
| if current and current_size + block.size > max_executables: |
There was a problem hiding this comment.
This is basically the majority of the logic, no? If A + next can fit, use it. Else, continue.
3 participants
Introduce
splitmethod toProgramSetto split program set into a list of program sets each with at most an allowed number of executables.Also fixed coverage output when running
tox.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.