Skip to content

Conversation

EarlMilktea
Copy link
Collaborator

@EarlMilktea EarlMilktea commented Jul 11, 2025

This PR implements infer_layer, which infers flow layer from input flow mapping.
This function is expected to replace xflow_from_pattern features in graphix .

Minor Changes

  • Add some validations
  • Add suboptimal flow/gflow verification

@EarlMilktea EarlMilktea requested a review from Copilot July 11, 2025 07:55
@EarlMilktea EarlMilktea self-assigned this Jul 11, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2025

Codecov Report

❌ Patch coverage is 99.59677% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.99%. Comparing base (62ff724) to head (43c1bce).

Files with missing lines Patch % Lines
python/swiflow/common.py 98.50% 0 Missing and 1 partial ⚠️
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     
Flag Coverage Δ
python 99.73% <99.41%> (-0.27%) ⬇️
rust 93.78% <100.00%> (+0.64%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Copilot

This comment was marked as outdated.

EarlMilktea and others added 2 commits July 11, 2025 17:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@EarlMilktea EarlMilktea changed the title Improve UX Implement layer inference Jul 22, 2025
@EarlMilktea EarlMilktea marked this pull request as ready for review July 22, 2025 09:57
Copilot

This comment was marked as outdated.

Copy link
Contributor

@thierry-martinez thierry-martinez left a comment

Choose a reason for hiding this comment

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

  • I would not expect to require the graph to be connected: we would like the flow algorithms to work even for graphs with multiple connected components.
  • What infer_layer computes is unclear, and it rejects some flows that appear to be valid (such as some flows returned by pflow.find).
  • The implemented notion of ensure_optimal is less strict than the maximally-delayed condition.


def infer_layer(g: nx.Graph[_V], anyflow: Mapping[_V, _V | AbstractSet[_V]]) -> Mapping[_V, int]:
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I would expect the name to be infer_layers, since there are potentially multiple layers.
  • Is infer_layer supposed to find the layers for any flow returned by find_pflow? I got an error with the following test, that finds a pflow and then try to find the layers:
from __future__ import annotations

from math import pi
from typing import assert_never

from graphix import Circuit
from graphix.instruction import CNOT, RY
from graphix.measurements import Measurement, PauliMeasurement
from graphix.fundamentals import Axis, Plane
from graphix.opengraph import OpenGraph

from swiflow import common
from swiflow.common import PPlane
import swiflow.pflow


def measurement_to_pplane(measurement: Measurement) -> PPlane:
    if pm := PauliMeasurement.try_from(measurement.plane, measurement.angle):
        if pm.axis == Axis.X:
            return PPlane.X
        elif pm.axis == Axis.Y:
            return PPlane.Y
        elif pm.axis == Axis.Z:
            return PPlane.Z
    if measurement.plane == Plane.XY:
        return PPlane.XY
    if measurement.plane == Plane.XZ:
        return PPlane.XZ
    if measurement.plane == Plane.YZ:
        return PPlane.YZ
    assert_never(measurement.plane)


def test_infer_layer() -> None:
    circuit = Circuit(
        width=2,
        instr=[
            CNOT(0, 1),
            RY(0, pi / 4),
        ],
    )
    pattern = circuit.transpile().pattern
    og = OpenGraph.from_pattern(pattern)
    meas_pplanes = {
        node: measurement_to_pplane(measurement)
        for node, measurement in og.measurements.items()
    }
    pflow, _layers = swiflow.pflow.find(
        og.inside, set(og.inputs), set(og.outputs), pplane=meas_pplanes
    )
    assert pflow is not None
    _ilayers = common.infer_layer(
        og.inside, pflow
    )  # raises ValueError: Failed to determine layer for all nodes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

infer_layers

Do you come up with another name that better incorporates its nature of returning only one candidate?
I admit current name is not the best.

any flow returned by find_pflow

Currently pflow is not supported (this is why pflow unit test is missing!)
I suppose we need to drop some order constraints from the input flow, specifically from where Pauli-specific MC simplifications are exploited: I have an idea but not implemented for now.
Could you help me with debugging if I suggest in this PR?

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Jul 24, 2025

Choose a reason for hiding this comment

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

Added ee1ae77 .
In _special_edges , all the exceptional edges (u, v) where v is either

  • X -measured + in f[u] : X in the stabilizer is absorbed into M^X
  • Z -measured + in Odd(f[u]) : Z in the stabilizer is absorbed into M^Z
  • Y -measured + in both f[u] and Odd(f[u]) : overall contributions to v reduces to Y absorbed into M^Y

are enumerated and later these edges are pruned from partial order constraints.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you come up with another name that better incorporates its nature of returning only one candidate?

infer_layers is fine, since we expect there will be only one returned value, but if you want a singular noun, you can choose infer_layering.

Added ee1ae77 .

There is still an issue with [CNOT(0, 1), CNOT(0, 1), RY(0, pi / 4)].

Could you help me with debugging if I suggest in this PR?

Of course, but I first would like to know why we need such a function infer_layer* and why we want to implement it in swiflow if it's pure Python.

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Jul 26, 2025

Choose a reason for hiding this comment

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

we want to implement it in swiflow if it's pure Python

Main reason is to align verification API to the graphix's in a locally-closed way.
(MEMO: I'm now planning to automatically call infer_layer inside verify_xxx if missing, and make infer_layer private as it's usage is a bit tricky.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MEMO:

infer_layer is necessary because xxxflow_from_pattern in graphix proceeds as follows:

  1. Identify M -> C dependencies
  2. Translate it to a corresponding flow-like
  3. Return it if valid

So we somehow complement missing layer information.
infer_layer is the generalization of related functionalities implemented in current graphix .

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 also updated the name from infer_layer to infer_layers in aeb0e8a .

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, since the flow_from_pattern functions in Graphix return a pair consisting of a flow and the corresponding minimal layering, they need to compute this layering. Note, however, that the layering is not part of the formal definition of a flow. It just so happens that flow-finding algorithms often compute it as a side product, so it is natural to return it as well, since it has already been computed.

However, one could imagine not computing the layering if it is not obtained for free as a side product, and instead letting the user compute it on demand by explicitly calling infer_layers. In particular, I would expect find_flow to return the layering in addition to the flow, but not flow_from_pattern, since the latter does not necessarily need to.

In any case, we were previously discussing the verify_flow functions, and I still don't see the role of the layering in these functions. More precisely, there are two kinds of flow verification:

  • Given an open graph and a flow, verify that the flow is well-formed;

  • Given a pattern and a flow, verify that the pattern implements the given flow.

In both cases, the layering does not play any role.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

layering does not play any role

Reasonable. I will update verification APIs based on this idea.
Yet, I believe we should keep full-verification features (verify both geometric and order requirements) in swiflow as they're the most straightforward implementation of the original papers.
Of course, their visibility (whether users can access to them or not) is a different problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thierry-martinez

Now layer-related checks are skipped if missing.

Comment on lines 105 to 106
ensure_optimal : `bool`
Whether the pflow should be maximally-delayed. Defaults to `False`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the exact condition you would like to check for ensure_optimal?
Currently, validate::check_initial only checks that the layer 0 is the layer of the output nodes. The following example delays non-output nodes in successive useless layers without being caught by ensure_optimal:

from typing import assert_never

from graphix import Circuit
from graphix.measurements import Measurement, PauliMeasurement
from graphix.opengraph import OpenGraph
from graphix.fundamentals import Axis, Plane
from swiflow.common import PPlane
import swiflow.pflow


def measurement_to_pplane(measurement: Measurement) -> PPlane:
    if pm := PauliMeasurement.try_from(measurement.plane, measurement.angle):
        if pm.axis == Axis.X:
            return PPlane.X
        elif pm.axis == Axis.Y:
            return PPlane.Y
        elif pm.axis == Axis.Z:
            return PPlane.Z
    if measurement.plane == Plane.XY:
        return PPlane.XY
    if measurement.plane == Plane.XZ:
        return PPlane.XZ
    if measurement.plane == Plane.YZ:
        return PPlane.YZ
    assert_never(measurement.plane)


def test_optimal():
    circuit = Circuit(2)
    circuit.h(0)
    circuit.cnot(0, 1)
    pattern = circuit.transpile().pattern
    og = OpenGraph.from_pattern(pattern)
    inputs = set(og.inputs)
    outputs = set(og.outputs)
    meas_pplanes = {
        node: measurement_to_pplane(measurement)
        for node, measurement in og.measurements.items()
    }
    pflow, layers = swiflow.pflow.find(og.inside, inputs, outputs, pplane=meas_pplanes)
    layers = {node: node + 1 if layer > 0 else 0 for node, layer in layers.items()}
    swiflow.pflow.verify(
        (pflow, layers),
        og.inside,
        inputs,
        outputs,
        pplane=meas_pplanes,
        ensure_optimal=True,
    )

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 may need to first "standardize" layers to...

  • use 0 as the minimum layer
  • use contiguous integers

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 completely forgot about minimum layer verification already added in this PR.
As min(layers) == 0 is guaranteed and validate::check_initial is the only place where the value of layer[u] itself matters (only order statistics are tested in other occurrences), the above behavior you reported is not a bug, i.e., my code correctly recognizes 1 is not used and thus the input flow has minimum number of distinct layers.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the example I provided, layers is {0: 1, 1: 2, 2: 0, 3: 4, 4: 0}, that is, there are 4 different layers: 0, 1, 2, and 4, rather than just 2 layers (the original value for layers is {0: 1, 1: 1, 2: 0, 3: 1, 4: 0}).

Copy link
Contributor

@shinich1 shinich1 Jul 26, 2025

Choose a reason for hiding this comment

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

Let's not call it optimal because optimality depends on context (Mhalla&Perdrix paper say optimal flow but this only means the depth is minimal; there are many ways for things to be optimal depending on your requirement). how about being more explicit like ensure_maximally_delayed)

Copy link
Contributor

Choose a reason for hiding this comment

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

The function infer_layers does not compute any flow (maximally-delayed or otherwise); instead, it takes a flow as input and returns a layering corresponding to that flow. The layering returned by infer_layers is minimal in the sense that there is no other layering compatible with the given flow that assigns a strictly smaller layer to any node.

However, this is not directly related to the notion of a maximally-delayed flow, since the function is defined for any flow, whether maximally-delayed or not. It just so happens that the layering associated with a maximally-delayed flow is pointwise less than the layering for any other flow, but infer_layers simply computes the layering for the specific flow it is given and does not attempt to find the maximally-delayed one.

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Jul 28, 2025

Choose a reason for hiding this comment

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

the function is defined for any flow

Oh I forgot about this.
Then an appropriate wording here might be "infer_layers minimizes the number of distinct layers" and the word "maximally-delayed" should not be included at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

The function infer_layers returns the layering that is pointwise minimal among all the layerings compatible with the given flow, in the sense given above: for every node $n$, there is no other layering compatible with the given flow that assigns a strictly smaller layer to $n$.

Copy link
Contributor

Choose a reason for hiding this comment

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

(In particular, it minimizes the number of distinct layers; however the characterization in terms of pointwise minimality is stronger.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now ensure_optimal is removed.

Copy link

@Copilot Copilot AI left a 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_layer function 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 plike is ambiguous. Consider using a more descriptive name like plane_mapping or measurement_planes to 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)

Copy link
Contributor

@thierry-martinez thierry-martinez left a 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.

Comment on lines +83 to +90
def _codec_wrap(
codec: IndexMap[_V],
pflow: tuple[_PFlow[_V], _Layer[_V]] | _PFlow[_V],
) -> tuple[dict[int, set[int]], list[int] | None]:
if isinstance(pflow, tuple):
f, layers = pflow
return codec.encode_gflow(f), codec.encode_layers(layers)
return codec.encode_gflow(pflow), None
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is identical to gflow._codec_wrap. Can we introduce a shared implementation in _common?

Comment on lines +143 to +145
if let Some(layers) = layers {
check_def_layer(&f, &layers, &g)?;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If no layering is given, we should still check that there is a partial order compatible with the flow. You can either use infer_layers for that, or just check that the dependency graph forms a DAG. The same remark applies for the other verify functions.

import networkx as nx

import swiflow.flow

def test_verify() -> None:
    g = nx.Graph([(0, 2), (1, 2)])
    iset = {0, 1}
    oset = {2}
    # Expected: a pattern where |I| > |O| cannot admit a flow
    assert swiflow.flow.find(g, iset, oset) == None
    flow = {0: 2, 1: 2}
    # Unexpected success: this should be rejected because:
    # - f(0) = 2 implies that 0 ≺ 1, since 1 ∈ N(2)
    # - f(1) = 2 implies that 1 ≺ 0, since 0 ∈ N(2)
    # but we cannot have both 0 ≺ 1 and 1 ≺ 0.
    swiflow.flow.verify(flow, g, iset, oset)

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Jul 31, 2025

Choose a reason for hiding this comment

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

Isn't it contradicting this thread #18 (comment) ?
I would like to know whether you later noticed it is actually necessary (layer does play an important role), or I'm still missing some bits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, sorry, I should soften my statement:

  • In the context of flow_from_pattern (the thread of the comment you cited), the corrections don’t play any role when verifying that a pattern implements a given flow.

  • In the context of checking a flow with respect to an open graph (the current thread, which I mistakenly invoked in the comment you cited), the order of the corrections is only relevant because we need to check that it forms a DAG. Using infer_layers is one way to verify that, although it computes more than we actually need here.

/// - XY: i not in g(i) and in Odd(g(i))
/// - YZ: i in g(i) and in Odd(g(i))
/// - XZ: i in g(i) and not in Odd(g(i))
fn check_def_geom(f: &GFlow, g: &[Nodes], planes: &Planes) -> Result<(), FlowValidationError> {
for &i in itertools::chain(f.keys(), planes.keys()) {
if f.contains_key(&i) != planes.contains_key(&i) {
Err(InvalidMeasurementSpec { node: i })?;
}
}
for (&i, fi) in f {
let pi = planes[&i];
Copy link
Contributor

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 $f(i) \in \mathcal P(I^c)$, and this condition should be checked. (Courtesy of @matulni for the counter-example.)

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)

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Aug 5, 2025

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 $f(i) \in \mathcal P(I^c)$ .
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 $f(i) \notin \mathcal P(I^c)$ .
I welcome your comments on this observation.

Copy link
Contributor

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.

Copy link
Collaborator Author

@EarlMilktea EarlMilktea Aug 5, 2025

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 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants