Skip to content
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

Restrict Circuit to HUGRs with valid roots #112

Closed
aborgna-q opened this issue Sep 15, 2023 · 1 comment · Fixed by #370
Closed

Restrict Circuit to HUGRs with valid roots #112

aborgna-q opened this issue Sep 15, 2023 · 1 comment · Fixed by #370
Assignees
Labels
enhancement New feature or request

Comments

@aborgna-q
Copy link
Collaborator

Instead of a blanket implementation on T: HugrView.
This will probably need some dark type magic if we want to keep a clean API.

@acl-cqc
Copy link
Contributor

acl-cqc commented Sep 18, 2023

I suggest a Hugr-side struct RootView(&Hugr) that implements HugrView - either via delegate! or by AsRef - and whose constructor checks the root type of the hugr in the same way as SiblingGraph::try_new. Then it can set HugrView's type Root and you can define, or at least impl, something like Circuit<T: HugrView<Root=DfgId>>

@aborgna-q aborgna-q self-assigned this May 28, 2024
@aborgna-q aborgna-q changed the title Restrict Circuit to HugrViews with valid Roots ? Restrict Circuit to HUGRs with valid roots May 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 3, 2024
Replaces the `trait Circuit : HugrView` with a struct containing a `T:
HugrView` and a node id indicating the container node for the circuit in
the hugr.

There are a few design going into this definition:

- #### Struct with checked parent node type.
The previous `Circuit` trait asked the user to only use it for
dataflow-based hugrs. This of course lead to functions assuming things
about the hugr structure and panicking with random errors when passed an
otherwise valid hugr.
  
Bar checking the root node at each method call, this can only be solved
with an opaque struct that checks its preconditions at construction
time.
  
The code here will throw a user-friendly error when trying to use
incompatible hugrs:
  ```
Node(0) cannot be used as a circuit parent. A Module is not a dataflow
container.
  ```

- #### Generic `T` defaulting to `Hugr`
Most uses in this library manipulate circuits as owned structures (e.g.
passing circuits around in badger).
However, sometimes we don't own the hugr so we cannot create a
`Circuit<Hugr>` from it. With the generics definition we can use
`Circuit<&Hugr>` or `Circuit<&mut Hugr>` to respectively view or modify
non-owned structures.
  
- #### Parent node pointer
Circuits represented as hugrs are not necessarily a flat dataflow
structure.
For example, a guppy-defined circuit may include calls to other methods
from the module.
By including a pointer to the entry point of the circuit we can keep all
this structure, and rewrite it latter as necessary.
This is also useful for non-owned hugrs; we can have a `Circuit<&Hugr>`
pointing to a region in the hugr without needing to modify anything
else.

Closes #112.

BREAKING CHANGE: Replaced the `Circuit` trait with a wrapper struct.

---------

Co-authored-by: Seyon Sivarajah <seyon.sivarajah@cambridgequantum.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants