Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0c4246f
add overwrite_hugr method
CalMacCQ Nov 17, 2025
63e120a
update ComposablePass impl to have a Hugr return type
CalMacCQ Nov 18, 2025
7481efb
Merge branch 'main' into cm/overwrite_hugr_method
CalMacCQ Nov 18, 2025
e83887c
apply some of Agustin's suggestions
CalMacCQ Nov 18, 2025
0e1ffa9
fix ComposedPass __call__ impl
CalMacCQ Nov 18, 2025
fe94fcc
Merge branch 'main' into cm/overwrite_hugr_method
CalMacCQ Nov 19, 2025
71a0a86
Apply suggestions from code review
CalMacCQ Nov 19, 2025
e29aafd
fix name
CalMacCQ Nov 19, 2025
1b6559c
add _apply and _apply_inplace for ComposedPass
CalMacCQ Nov 19, 2025
e56cd7d
Merge branch 'main' into cm/overwrite_hugr_method
CalMacCQ Nov 19, 2025
bfb6f26
apply suggestions from @acl-cqc
CalMacCQ Nov 19, 2025
8037052
docstring
CalMacCQ Nov 19, 2025
859c811
idea: Alternative to multiple ComposablePass apply methods
aborgna-q Nov 20, 2025
d79a031
feat: PassResult definition
aborgna-q Nov 20, 2025
b3eabde
Use pass names rather than objects, so the result is serializable
aborgna-q Nov 20, 2025
97e5406
More tests
aborgna-q Nov 20, 2025
07caa46
Add pass name to error message
aborgna-q Nov 20, 2025
a1eebb0
Update hugr-py/src/hugr/passes/_composable_pass.py
aborgna-q Nov 24, 2025
ad8ed71
if tree
aborgna-q Nov 24, 2025
44f5fa3
Overrite inplace param
aborgna-q Nov 24, 2025
21ee55a
typo
aborgna-q Nov 24, 2025
be1cad4
Merge remote-tracking branch 'origin/main' into ab/composed-pass-result
aborgna-q Nov 24, 2025
8b9b59b
post-merge cleanup
aborgna-q Nov 24, 2025
4c4cdcd
not my dummy
aborgna-q Nov 24, 2025
1e64469
s/impl_pass_run/implement_pass_run/
aborgna-q Nov 24, 2025
1794edc
s/pass_/composable_pass/
aborgna-q Nov 24, 2025
19662f5
typos
aborgna-q Nov 24, 2025
7cf550d
Replace `original_dirty` with `inplace` flag in results, explain flag…
aborgna-q Nov 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions hugr-py/src/hugr/hugr/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,14 @@ def insert_hugr(self, hugr: Hugr, parent: ToNode | None = None) -> dict[Node, No
)
return mapping

def _overwrite_hugr(self, new_hugr: Hugr) -> None:
"""Modify a Hugr in place by replacing contents with those from a new Hugr."""
self.module_root = new_hugr.module_root
self.entrypoint = new_hugr.entrypoint
self._nodes = new_hugr._nodes
self._links = new_hugr._links
self._free_nodes = new_hugr._free_nodes

def _to_serial(self) -> SerialHugr:
"""Serialize the HUGR."""

Expand Down
170 changes: 146 additions & 24 deletions hugr-py/src/hugr/passes/_composable_pass.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,84 @@

from __future__ import annotations

from dataclasses import dataclass
from typing import TYPE_CHECKING, Protocol, runtime_checkable
from copy import deepcopy
from dataclasses import dataclass, field
from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable

if TYPE_CHECKING:
from collections.abc import Callable

from hugr.hugr.base import Hugr


# Type alias for a pass name
PassName = str


@runtime_checkable
class ComposablePass(Protocol):
"""A Protocol which represents a composable Hugr transformation."""

def __call__(self, hugr: Hugr) -> None:
"""Call the pass to transform a HUGR."""
...
def __call__(self, hugr: Hugr, *, inplace: bool = True) -> Hugr:
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we have the impl_pass_run function as a helper for implementing ComposablePass.run where is the __call__ method actually used in the pass implementation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was a suggestion from Seyon. Most usecases just need the Hugr after the pass, so we provide a simple call method.

When we actually need to inspect the result/pass output we can use the other call.

"""Call the pass to transform a HUGR, returning a Hugr."""
return self.run(hugr, inplace=inplace).hugr

def run(self, hugr: Hugr, *, inplace: bool = True) -> PassResult:
"""Run the pass to transform a HUGR, returning a PassResult.

See :func:`_impl_pass_run` for a helper function to implement this method.
"""

@property
def name(self) -> str:
def name(self) -> PassName:
"""Returns the name of the pass."""
return self.__class__.__name__

def then(self, other: ComposablePass) -> ComposablePass:
"""Perform another composable pass after this pass."""
# Provide a default implementation for composing passes.
pass_list = []
if isinstance(self, ComposedPass):
pass_list.extend(self.passes)
else:
pass_list.append(self)

if isinstance(other, ComposedPass):
pass_list.extend(other.passes)
else:
pass_list.append(other)

return ComposedPass(pass_list)
return ComposedPass(self, other)


def impl_pass_run(
Copy link
Collaborator

Choose a reason for hiding this comment

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

To me this name feels too "internal". I know it's inside an underscored module but when we stabilize it we may want a better name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed it to "implement_pass_run".
Not the best either, but hopefully a bit less internal-sounding

pass_: ComposablePass,
*,
hugr: Hugr,
inplace: bool,
inplace_call: Callable[[Hugr], PassResult] | None = None,
copy_call: Callable[[Hugr], PassResult] | None = None,
) -> PassResult:
"""Helper function to implement a ComposablePass.run method, given an
inplace or copy-returning pass methods.

At least one of the `inplace_call` or `copy_call` arguments must be provided.

:param pass_: The pass being run. Used for error messages.
:param hugr: The Hugr to apply the pass to.
:param inplace: Whether to apply the pass inplace.
:param inplace_call: The method to apply the pass inplace.
:param copy_call: The method to apply the pass by copying the Hugr.
:return: The result of the pass application.
:raises ValueError: If neither `inplace_call` nor `copy_call` is provided.
"""
if inplace and inplace_call is not None:
return inplace_call(hugr)
elif inplace and copy_call is not None:
pass_result = copy_call(hugr)
pass_result.hugr = hugr
if pass_result.modified:
hugr._overwrite_hugr(pass_result.hugr)
pass_result.original_dirty = True
return pass_result
elif not inplace and copy_call is not None:
return copy_call(hugr)
elif not inplace and inplace_call is not None:
new_hugr = deepcopy(hugr)
pass_result = inplace_call(new_hugr)
pass_result.original_dirty = False
return pass_result
else:
msg = f"{pass_.name} needs to implement at least an inplace or copy run method"
raise ValueError(msg)


@dataclass
Expand All @@ -48,11 +91,90 @@ class ComposedPass(ComposablePass):

passes: list[ComposablePass]

def __call__(self, hugr: Hugr):
"""Call all of the passes in sequence."""
for comp_pass in self.passes:
comp_pass(hugr)
def __init__(self, *passes: ComposablePass) -> None:
self.passes = []
for pass_ in passes:
if isinstance(pass_, ComposedPass):
self.passes.extend(pass_.passes)
else:
self.passes.append(pass_)

def run(self, hugr: Hugr, *, inplace: bool = True) -> PassResult:
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually think you can just do:

def run(self, hugr: Hugr, *, inplace: bool = True) -> PassResult:
  pass_result = PassResult(hugr=hugr)
  for pass_ in self.passes:
    new_result = pass_.run(pass_result.hugr, inplace=inplace)
    pass_result = pass_result.then(new_result)
  return pass_result

Because pass_.run always returns a PassResult containing the result hugr; we don't care whether that's the input hugr or a fresh one.

Copy link
Collaborator Author

@aborgna-q aborgna-q Nov 24, 2025

Choose a reason for hiding this comment

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

That's already the case, or I'm missing something?

def apply(hugr: Hugr) -> PassResult:
pass_result = PassResult(hugr=hugr)
for comp_pass in self.passes:
new_result = comp_pass.run(pass_result.hugr, inplace=inplace)
pass_result = pass_result.then(new_result)
return pass_result

return impl_pass_run(
self,
hugr=hugr,
inplace=inplace,
inplace_call=apply,
Copy link
Contributor

Choose a reason for hiding this comment

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

inplace_call and copy_call are the same function here, is that right? Either of them will capture local variable inplace: bool = True specified in the call to run.

Ah ok. I think the one that's going to be used will have the correct semantics, and the callback that isn't going to be used....had better not be, it will not work correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added explicit flag overrides

copy_call=apply,
)

@property
def name(self) -> str:
def name(self) -> PassName:
return f"Composed({ ', '.join(pass_.name for pass_ in self.passes) })"


@dataclass
class PassResult:
"""The result of a series of composed passes applied to a HUGR.

Includes a flag indicating whether the passes modified the HUGR, and an
arbitrary result object for each pass.

In some cases, `modified` may be set to `True` even if the pass did not
modify the program.

:attr hugr: The transformed Hugr.
:attr original_dirty: Whether the original HUGR was modified by the pass.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is "dirty" the right word here? The meaning of original_dirty and modified look very similar. I don't actually see the point of the original_dirty flag. Presumably the user either knows or doesn't care whether the pass was called in-place.

Copy link
Collaborator Author

@aborgna-q aborgna-q Nov 24, 2025

Choose a reason for hiding this comment

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

Replaced original_dirty with just an inplace flag.
I think it's nice to have a flag assuring you whether the hugr got copied or not, but it's not super important.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. We also rule out any optimization that wants to destroy the input Hugr in the process of constructing an all-new output Hugr. I mean, ruling that out might be good ;)

:attr modified: Whether the pass made changes to the HUGR.
Copy link
Contributor

@acl-cqc acl-cqc Nov 24, 2025

Choose a reason for hiding this comment

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

Ok, so if inplace = True,

  • original_dirty == modified, and the transformed Hugr is the input Hugr

whereas if inplace == False,

  • original_dirty should always be False
  • modified indicates whether the output Hugr == the input
  • (probably but not necessarily) the output never is the input. (We might want to allow the latter if modified is False)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

modified indicates whether the output Hugr == the input; but (probably, not necessarily) the output never is the input. (We might want to allow the latter if modified is False)

Conditionally aliasing the output is a bug waiting to happen. If we say the output is a copying the object then we should always do that.
Otherwise we may start modifying the result and unintentionally changing the original too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. Ok, so - if inplace:

  • original_dirty == modified
  • output Hugr is input Hugr

if not inplace:

  • original_dirty == False? (Right? We don't allow it to be partially invalidated or anything)
  • (output Hugr is input Hugr) is always False
  • modified indicates whether the output Hugr == the input

:attr results: The result of each applied pass, as a tuple of the pass and
the result.
"""

hugr: Hugr
original_dirty: bool = False
modified: bool = False
results: list[tuple[PassName, Any]] = field(default_factory=list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why we include the pass name here?

Copy link
Collaborator Author

@aborgna-q aborgna-q Nov 24, 2025

Choose a reason for hiding this comment

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

Mostly for debuggability, otherwise ComposedPass.run would return a List[Any] of arbitrary payloads without much indication of what came from where.

The result may also be serialized, so the original ComposedPass and its list of passes may be lost.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought including the pass name was quite a nice solution!


@classmethod
def for_pass(
cls,
pass_: ComposablePass,
hugr: Hugr,
*,
result: Any,
inline: bool,
modified: bool = True,
) -> PassResult:
"""Create a new PassResult after a pass application.

:param hugr: The Hugr that was transformed.
:param pass_: The pass that was applied.
:param result: The result of the pass application.
:param inline: Whether the pass was applied inplace.
:param modified: Whether the pass modified the HUGR.
"""
return cls(
hugr=hugr,
original_dirty=inline and modified,
modified=modified,
results=[(pass_.name, result)],
)

def then(self, other: PassResult) -> PassResult:
"""Extend the PassResult with the results of another PassResult.

Keeps the hugr returned by the last pass.
"""
return PassResult(
hugr=other.hugr,
original_dirty=self.original_dirty or other.original_dirty,
modified=self.modified or other.modified,
results=self.results + other.results,
)
111 changes: 95 additions & 16 deletions hugr-py/tests/test_passes.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,105 @@
from copy import deepcopy

import pytest

from hugr.hugr.base import Hugr
from hugr.passes._composable_pass import ComposablePass, ComposedPass
from hugr.passes._composable_pass import (
ComposablePass,
ComposedPass,
PassResult,
impl_pass_run,
)


def test_composable_pass() -> None:
class MyDummyPass(ComposablePass):
def __call__(self, hugr: Hugr) -> None:
return self(hugr)
class MyDummyInlinePass(ComposablePass):
def run(self, hugr: Hugr, inplace: bool = True) -> PassResult:
return impl_pass_run(
self,
hugr=hugr,
inplace=inplace,
inplace_call=lambda hugr: PassResult.for_pass(
self,
hugr,
result=None,
inline=True,
# Say that we modified the HUGR even though we didn't
modified=True,
),
)

dummy = MyDummyPass()
class MyDummyCopyPass(ComposablePass):
def run(self, hugr: Hugr, inplace: bool = True) -> PassResult:
return impl_pass_run(
self,
hugr=hugr,
inplace=inplace,
copy_call=lambda hugr: PassResult.for_pass(
self,
deepcopy(hugr),
result=None,
inline=False,
# Say that we modified the HUGR even though we didn't
modified=True,
),
)

composed_dummies = dummy.then(dummy)
dummy_inline = MyDummyInlinePass()
dummy_copy = MyDummyCopyPass()

my_composed_pass = ComposedPass([dummy, dummy])
assert my_composed_pass.passes == [dummy, dummy]
composed_dummies = dummy_inline.then(dummy_copy)
assert isinstance(composed_dummies, ComposedPass)

assert isinstance(composed_dummies, ComposablePass)
assert composed_dummies == my_composed_pass
assert dummy_inline.name == "MyDummyInlinePass"
assert dummy_copy.name == "MyDummyCopyPass"
assert composed_dummies.name == "Composed(MyDummyInlinePass, MyDummyCopyPass)"
assert composed_dummies.then(dummy_inline).then(composed_dummies).name == (
"Composed("
+ "MyDummyInlinePass, MyDummyCopyPass, "
+ "MyDummyInlinePass, "
+ "MyDummyInlinePass, MyDummyCopyPass)"
)

assert dummy.name == "MyDummyPass"
assert composed_dummies.name == "Composed(MyDummyPass, MyDummyPass)"
# Apply the passes
hugr: Hugr = Hugr()
new_hugr = composed_dummies(hugr, inplace=False)
assert hugr == new_hugr
assert new_hugr is not hugr

assert (
composed_dummies.then(my_composed_pass).name
== "Composed(MyDummyPass, MyDummyPass, MyDummyPass, MyDummyPass)"
)
# Verify the pass results
hugr = Hugr()
inplace_result = composed_dummies.run(hugr, inplace=True)
assert inplace_result.modified
assert inplace_result.original_dirty
assert inplace_result.results == [
("MyDummyInlinePass", None),
("MyDummyCopyPass", None),
]
assert inplace_result.hugr is hugr

hugr = Hugr()
copy_result = composed_dummies.run(hugr, inplace=False)
assert copy_result.modified
assert not copy_result.original_dirty
assert copy_result.results == [
("MyDummyInlinePass", None),
("MyDummyCopyPass", None),
]
assert copy_result.hugr is not hugr


def test_invalid_composable_pass() -> None:
class MyDummyInvalidPass(ComposablePass):
def run(self, hugr: Hugr, inplace: bool = True) -> PassResult:
return impl_pass_run(
self,
hugr=hugr,
inplace=inplace,
)

dummy_invalid = MyDummyInvalidPass()
with pytest.raises(
ValueError,
match="MyDummyInvalidPass needs to implement at least an inplace or copy run method", # noqa: E501
):
dummy_invalid.run(Hugr())
Loading