feat(py): a ComposablePass protocol for hugr-py

[CalMacCQ]

closes #2617

Mostly follows Agustin's suggestion see eeb4c50

I'm also added some to/from_dict methods as well as a supported_ops property. EDIT: removed these additional methods for now.

[aborgna-q]

We won't support encoding/decoding every pass, as some may have non encodable context.

Should we do a best effort to_dict encoding, and signal when the result is not complete? (so handlers downstream can get some partial info about it), or outright error-out if not supported?

For the from_dict implementation, we'll need some kind of pass registry that knows a set of passes it can decode. We can think about that later.

[CalMacCQ]

What would be an example of a pass with a non-encodable context? A user defined pass where the user passes a function into the builder?

[CalMacCQ]

Also do you think I should remove the to/from_dict for now? Not sure pass serialisation belongs in the initial work for Guppy optimisation.

We should add it at some point for nexus integration though I think.

[aborgna-q]

Yeah, I think it needs more design discussion.

I'm also guessing that we'd want to return a dataclass rather than an arbitrary dict, to keep some structure...

[CalMacCQ]

Yes, good point. I'll remove it for now.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.37%. Comparing base (0df2d77) to head (45bd86a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2636   +/-   ##
=======================================
  Coverage   83.37%   83.37%           
=======================================
  Files         259      260    +1     
  Lines       50703    50708    +5     
  Branches    46264    46264           
=======================================
+ Hits        42275    42280    +5     
  Misses       6061     6061           
  Partials     2367     2367           

Flag	Coverage Δ
python	91.47% <100.00%> (+<0.01%)	⬆️

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.

[CalMacCQ]

After chatting to Alec it may be worth removing the supported_ops property. In TKET1 having a similar property for supported gates causes issues when we compose passes with mismatching GateSetPredicates. Its been a bit of a maintence headache. What do you think @aborgna-q?

[aborgna-q]

it may be worth removing the supported_ops property

Makes sense. Passes are more generic here than just rewrites over specific sets of gates, so fixing a closed set of optypes here seems counter-productive.

[CalMacCQ]

Using typing_extensions rather than typing as the Self type was introduced in Python 3.11 and we still support 3.10.

[CalMacCQ]

Open to better suggestions for this module name. I opted to not use pass.py as pass is a keyword in Python.

[aborgna-q]

LGTM
Though maybe I'd move it out of hugr.hugr, and into its own thing?
hugr.passes.ComposablePass?

[CalMacCQ]

sorry for commit spam. Was undoing some changes made by my editor.

[acl-cqc]

Looks good thank you @CalMacCQ. The main issue I remember from the hugr_passes ComposablePass was wanting to pass a single runtime boolean (whether to validate or not) around all the passes...which could be plumbed in, but we didn't.

In Python...I guess we could add an options: Options | None = None parameter to run, and indeed, we could do that later, so probably no real need to do that now.

[acl-cqc]

What does this actually mean? In particular in conjunction with is_global, I guess?

is_global == True might mean, transforms across the whole Hugr (or whole entrypoint subregion), in which case, "is_recursive" may be meaningless?

If is_global == False, perhaps that means it only affects the immediate children of the entrypoint (so the entrypoint identifies a flat circuit), in which case, recursive could mean, is also automatically applied to subcircuits beneath the entrypoint, fair enough.

But I wonder whether it'd be better to combine these into an enum-like thing where we can be more specific (and extensible) about what the possible variants mean?

[acl-cqc]

For some transformations (this is perhaps getting a bit complex so maybe never mind) there are actually two variables re. scope:

1. how much of the Hugr to analyze (without changing) - the more you analyze, the better/more accurate your results, but it takes more time and/or memory
2. how much of the Hugr to transform/change. I.e. how disruptive it can be.

(1.) probably needs to be a superset of (2.), but you might analyze the whole Hugr in order to change only one function, say (and this would enable more optimization in that function, than if you only analyzed that function alone, but the latter would be cheaper).

[CalMacCQ]

I think it should be

def with_entrypoint(self, hugr: Hugr, entrypoint: Node) -> Self: ...

rather than

def with_entrypoint(self, entrypoint: Node) -> Self: ...

Implementing the with_entrypoint method could also be problematic for reasons I dicussed with Agustin earlier.

As far as I know there isn't a way to get the Hugr subgraph given a entrypoint Node in Python. I think in rust there is something like SiblingSubgraph::try_from_nodes

Agustin suggested that we could modify the entrpoint before we apply the pass and then change it back?

def with_entrypoint(self, hugr: Hugr, entrypoint: Node) -> Self:
    original_entrypoint = hugr.entrypoint
    hugr.entrypoint = entrypoint

    # apply some transformation pass 

    hugr.entrypoint = orignal entrypoint
    return something

Not immediately obvious to me how this would work as we aren't returing an object of type Self. Maybe we could do something with a context manager here?