CQCL / hugr

Hierarchical Unified Graph Representation for quantum and classical programs
https://crates.io/crates/hugr
Apache License 2.0
18 stars 5 forks source link

Helper method to compare `FunctionType`s without checking extension sets. #1209

Closed aborgna-q closed 1 month ago

aborgna-q commented 3 months ago

The PartialEq implementation of FunctionType checks that the extension_reqs of both signatures is the same, in addition to the input/output types.

I find myself writing

if s1.input == other.input && s1.output == other.output { ... }

multiple times, when I don't care about mismatching extensions.

We could add a FunctionType::same_types(&self, other) -> bool method or something similar for these comparisons.

acl-cqc commented 2 months ago

I wonder if we should just impl Eq for FunctionType and ignore the delta when the extension_inference feature is off?

aborgna-q commented 2 months ago

It's not only about extension_inference.

In tket2 we have multiple occurrences where we don't care about the extensions used inside a definition but only about the actual types coming in an out.

This is normally the case when validating rewrite operations. E.g. https://github.com/CQCL/tket2/blob/ed263d52ab666e709787da5820124c57678ebecb/tket2-py/src/passes/chunks.rs#L58-L59

acl-cqc commented 2 months ago

Hmmmm, in that case the thing you are comparing against really isn't a(nother) FunctionType, but a (TypeRow, TypeRow) or just two TypeRows, right?

acl-cqc commented 2 months ago

So you could add FunctionType::get_io(&self) -> (&TypeRow, &TypeRow), say, and then....whether you wanted equal_io(&self, other: (&TypeRow, &TypeRow)) as well, I dunno

aborgna-q commented 2 months ago

Sure, get_io would work and may come useful in more general cases.

acl-cqc commented 2 months ago

For a rewrite, we'd expect the replacement to have (same or fewer) requirements, right? So other.inputs == self.inputs && other.outputs == self.outputs && other.extension_reqs.is_subset(self.extension_reqs) ? That could be a method itself...

But might lead to all sorts of awkward questions about covariance/contravariance - what if the return type is a (higher-order) function (value), are we allowed to reduce the extension requirements of that, etc.... get_io (probably) avoids anyone worrying about that :)

aborgna-q commented 2 months ago

we'd expect the replacement to have (same or fewer) requirements

Not necessarily. The example I wrote is part of stitching chunks of a circuit back after being updated. We can do whatever we want with those chunks as long as the signatures(' rows!) match.

acl-cqc commented 2 months ago

we'd expect the replacement to have (same or fewer) requirements

Not necessarily. The example I wrote is part of stitching chunks of a circuit back after being updated. We can do whatever we want with those chunks as long as the signatures(' rows!) match.

If this is post-extension-inference, and you increase the delta, you'll have to update the parent/ancestors (unless they were already loose-upper-bounds that included the new delta!), which could be non-trivial (e.g. updating a FuncDefn requires recursively increasing delta of Calls). So, be warned....(a separate issue though)