Fix #193: add Pattern.check_runnability
#364
Conversation
This commit adds the method `Pattern.check_runnability` that ensures a pattern is runnable. This covers TeamGraphix#193, given that determinism is already checkable with Pauli flow finding, and pattern concatenation is already implemented by `Pattern.compose`.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #364 +/- ##
==========================================
- Coverage 86.27% 86.25% -0.02%
==========================================
Files 44 44
Lines 6106 6163 +57
==========================================
+ Hits 5268 5316 +48
- Misses 838 847 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
matulni
left a comment
matulni
left a comment
There was a problem hiding this comment.
Looks good, thanks, just a couple of minor comments.
| def add(self, cmd: Command) -> None: | ||
| """Add command to the end of the pattern. | ||
| An MBQC command is an instance of :class:`graphix.command.Command`. | ||
| Parameters | ||
| ---------- | ||
| cmd : :class:`graphix.command.Command` | ||
| MBQC command. | ||
| """ | ||
| if cmd.kind == CommandKind.N: | ||
| if cmd.node in self.__output_nodes: | ||
| raise NodeAlreadyPreparedError(cmd.node) | ||
| raise RunnabilityError(cmd, cmd.node, RunnabilityErrorReason.AlreadyActive) | ||
| self.__n_node += 1 | ||
| self.__output_nodes.append(cmd.node) | ||
| elif cmd.kind == CommandKind.M: | ||
| if cmd.node not in self.__output_nodes: | ||
| raise RunnabilityError(cmd, cmd.node, RunnabilityErrorReason.AlreadyMeasured) | ||
| self.__output_nodes.remove(cmd.node) | ||
| self.__seq.append(cmd) |
There was a problem hiding this comment.
I would add a comment emphasising that this method dos not ensure a secure pattern generation (which has to be verified with :func:Pattern.check_runnability.
Some minor comment: if we add a measurement command on a node i which is not in the pattern, we get an error message M(i): node i is already measured. which is a bit misleading. One way to fix it could be to store the existing nodes in the pattern on the fly (which would also allow to ensure that we can only add E and C commands on existing nodes).
There was a problem hiding this comment.
Checking N and M here means that we don't run into problems with extract_graph, space_list and similar methods, but I do think that there may be some niche use for allowing unrunnable patterns (not sure what though).
If we leave this as is, I would suggest adding check_runnability to methods associated with using the pattern for computation (or at the very least a flag _is_runnable that is checked when using these methods that may display a warning). Specifically simulate_pattern and perform_pauli_measurements but may be worthwhile (I'm not confident there won't be errors, so I think it's worth checking) in minimize_space, parallelize_pattern and shift_signals.
There was a problem hiding this comment.
In 49616d8, I removed all the partial runnability checks from Pattern.add: I think we would have the choice either to perform a full runnability check in add to guarantee that Pattern objects are always runnable or, as it is done for now, to make no such guarantee and to provide a runnability checker instead, but having only a partial test was awkward. Moreover, having add implementing a part of the runnability check makes it difficult to write tests for check_runnability that were already handled by add, so it is more convenient to be able to let non-runnable patterns being built.
Since standardization and signal shifting could have turned non-runnable patterns into runnable ones (and hence could hide some code-logic errors), I added a call to check_runnability at the beginning of standardization and signal shifting, so in particular all methods that require the pattern to be standardized first check for runnability (minimize_space, parallelize_pattern, perform_pauli_measurements), and I added a check in simulate_pattern too, so that error messages are clearer and raised before starting the simulation.
emlynsg
left a comment
emlynsg
left a comment
There was a problem hiding this comment.
Looks great Thierry! No approval-blocking issues.
|
Checking runnability before standardizing enables me to catch a bug in |
3 participants
This commit adds the method
Pattern.check_runnabilitythat ensures a pattern is runnable. This covers #193, given that determinism is already checkable with Pauli flow finding, and pattern concatenation is already implemented byPattern.compose.