-
Notifications
You must be signed in to change notification settings - Fork 2
Implement layer inference #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18 +/- ##
==========================================
+ Coverage 95.35% 95.99% +0.64%
==========================================
Files 16 16
Lines 925 1024 +99
Branches 27 48 +21
==========================================
+ Hits 882 983 +101
+ Misses 43 40 -3
- Partials 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements layer inference functionality that allows computing flow layers from input flow mappings, replacing the need for xflow_from_pattern features in graphix. The implementation includes layer inference for all flow types (flow, gflow, pflow) with validation and suboptimal flow verification capabilities.
Key changes:
- Adds
infer_layerfunction to automatically compute layers from flow mappings - Modifies verification functions to support both optimal and suboptimal flows
- Updates test cases to verify both optimal and suboptimal scenarios
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
python/swiflow/_common.py |
Implements core infer_layer function and helper utilities for layer computation |
python/swiflow/flow.py |
Updates flow verification to support layer inference and optional optimality checks |
python/swiflow/gflow.py |
Updates gflow verification to support layer inference and optional optimality checks |
python/swiflow/pflow.py |
Updates pflow verification to support layer inference and optional optimality checks |
python/swiflow/common.py |
Simplifies type definitions by removing dataclass wrappers and using type aliases |
src/flow.rs |
Adds optimal parameter to verification function |
src/gflow.rs |
Adds optimal parameter to verification function |
src/pflow.rs |
Adds optimal parameter to verification function |
tests/test_*.py |
Updates tests to use new API and adds layer inference verification tests |
tests/assets.py |
Updates test data format and adds new test cases |
Comments suppressed due to low confidence (2)
python/swiflow/_common.py:53
- The parameter name
plikeis ambiguous. Consider using a more descriptive name likeplane_mappingormeasurement_planesto clarify its purpose.
def check_planelike(vset: AbstractSet[_V], oset: AbstractSet[_V], plike: Mapping[_V, _P]) -> None:
tests/test_common.py:157
- The test expects a specific error message pattern with
match=r".*determine.*"but the actual error message from the function is "Failed to determine layer for all nodes." Ensure the test regex pattern accurately matches the expected error message.
_common.infer_layer(g, flow)
thierry-martinez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose a more efficient implementation for infer_layers.
| } | ||
| } | ||
| for (&i, fi) in f { | ||
| let pi = planes[&i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the definition of a gflow, we should have
import math
import networkx as nx
import numpy as np
from graphix.fundamentals import Plane
from graphix.states import PlanarState
from graphix.generator import _gflow2pattern
import swiflow.gflow
import swiflow.common
def test_gflow() -> None:
graph = nx.Graph([(0, 1)])
inputs = {0}
outputs = {1}
planes_swiflow = {0: swiflow.common.Plane.XZ}
# Expecting `None`: this graph does not admit a flow
g, l_k = swiflow.gflow.find(graph, inputs, outputs, planes=planes_swiflow)
planes = {0: Plane.XZ}
angles = {0: 0.1}
pattern = _gflow2pattern(graph, angles, inputs, planes, g, l_k)
input_state = PlanarState(Plane.XY, 0.7)
# Checks that the pattern is deterministic (as a pattern with a gflow should be)
state_ref = pattern.simulate_pattern(input_state=input_state)
for _ in range(25):
state = pattern.simulate_pattern(input_state=input_state)
assert math.isclose(np.abs(np.dot(state.flatten().conjugate(), state_ref.flatten())), 1)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it locally: will push later.
By the way, I suspect pflow finding algorithm introduced in https://arxiv.org/abs/2109.05654 could violate
For example, in the first iteration of PauliFlowAux(V,Γ,I,λ,A,B,k) with B = O, forall u visits u in I and thus u can be included in Kxz and Kxy , possibly leading to
I welcome your comments on this observation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this side condition is not checked by the published algorithm and should be checked in addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it still necessary for "special edges" (see my python impl. in this PR for the terminology)?
I suppose we could safely apply C after some specific Pauli M inside I , as we do for V \ I .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now trying to port infer_layers and related stuff to Rust, as at least special edge detection must be made available in the binding layer.
6a06853 to
1254cd7
Compare
This PR implements
infer_layer, which infers flow layer from input flow mapping.This function is expected to replace
xflow_from_patternfeatures ingraphix.Minor Changes