feat(core): Put extension declarations in top level of Hugr
#1613
Assignees
Milestone
Comments
This was referenced Oct 24, 2024
github-merge-queue bot
pushed a commit
to CQCL/tket2
that referenced
this issue
Nov 7, 2024
Removes the guppy-specific and adds supports for loading functions packages and standalone hugrs. Temporarily keeps track of the required extensions for the hugr in an optional `Circuit::required_extensions` field until CQCL/hugr#1613 gets implemented. Fallbacks to a default set when loading bare hugrs. Note that storing a circuit with a non-root parent is currently an error. We'll need to store some pointer to the entrypoint on the hugr's metadata, and that'll require some serialization-stable path encoding. I'll open an issue for that . blocked-by: CQCL/hugr#1621. I'll remove the patch in cargo.toml once that gets released. drive-by: Use `circuit_hash` for the `PartialEq` implementation of circuits. The derived equality failed on graphs with different node indices. BREAKING CHANGE: Removed `load_guppy_*` methods. Use `Circuit::load_function_reader` instead.
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 25, 2024
Extensions are defined once and shared throughout `ExtensionRegistr`ies, `Package`s, and soon `Hugr`s (#1613). This _write-once then share around_ is a good usecase for `Arc`s, specially since the definitions are mostly read and rarely cloned. This is a requisite for #1613, to avoid cloning all extensions for each new hugr. BREAKING CHANGE: `ExtensionRegistry` and `Package` now wrap `Extension`s in `Arc`s.
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 25, 2024
Extensions are defined once and shared throughout `ExtensionRegistr`ies, `Package`s, and soon `Hugr`s (#1613). This _write-once then share around_ is a good usecase for `Arc`s, specially since the definitions are mostly read and rarely cloned. This is a requisite for #1613, to avoid cloning all extensions for each new hugr. BREAKING CHANGE: `ExtensionRegistry` and `Package` now wrap `Extension`s in `Arc`s.
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 27, 2024
This change was extracted from the work towards #1613. Now `OpDef`s and `TypeDef`s keep a `Weak` reference to their extension's `Arc`. This way we will be able to automatically set the extension requirements when adding operations, so we can get rid of `update_validate` and the explicit registries when building hugrs. To implement this, the building interface for `Extension`s is sightly modified. Once an `Arc` is built it cannot be modified without doing internal mutation. But we need the `Arc`'s weak reference to define the ops and types. Thankfully, we can use `Arc::new_cyclic` which provides us with a `Weak` ref at build time so we are able to define things as needed. This is wrapped in a new `Extension::new_arc` method, so the user doesn't need to think about that. BREAKING CHANGE: Renamed `OpDef::extension` and `TypeDef::extension` to `extension_id`. `extension` now returns weak references to the `Extension` defining them. BREAKING CHANGE: `Extension::with_reqs` moved to `set_reqs`, which takes `&mut self` instead of `self`. BREAKING CHANGE: `Extension::add_type` and `Extension::add_op` now take an extra parameter. See docs for example usage. BREAKING CHANGE: `ExtensionRegistry::register_updated` and `register_updated_ref` are no longer fallible.
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 2, 2024
Another noisy PR 🫠 Adds an `extension_ref: Weak<Extension>` field on `CustomType`, pointing to the defining extension. This is a continuation of #1719, and should be the last step before #1613. The main problem with adding the field is that types can no longer be `const`-initialized. That means that every definition for, for example, `const QB_T: Type` now has been changed to a runtime function. This mean that we can no longer use the static `type_row!` constructor when using custom types, and must resort to a `vec!` (and many `into()`s...). I think with this the `Cow` inside a type row is not that useful anymore, and it may be better to just drop it in favour of vecs, perhaps with internal `Arc`s (I'll open an issue about this). Benchmark comparison after this change (showing those with >5% change): ``` group base custom-type-refs ----- ---- ---------------- builder/simple_cfg 1.00 15.1±0.22µs ? ?/sec 1.25 18.9±0.19µs ? ?/sec builder/simple_dfg 1.00 1534.3±50.82ns ? ?/sec 1.76 2.7±0.04µs ? ?/sec circuit_roundtrip/json/0 1.00 8.1±0.12µs ? ?/sec 1.06 8.6±0.16µs ? ?/sec circuit_serialize/json/0 1.00 1627.5±24.52ns ? ?/sec 1.34 2.2±0.04µs ? ?/sec circuit_serialize/json/1 1.00 6.5±0.17µs ? ?/sec 1.10 7.2±0.09µs ? ?/sec singleton_subgraph/10 1.15 231.9±84.31ns ? ?/sec 1.00 201.3±4.53ns ? ?/sec ``` There's a slight increase in building time for small circuits, but most tests are unchanged / tend to 1.00 for larger sizes. --- Review help. This is a list of the interesting bits of this PR, lost inside the noise. - The trait `MakeOpDef` in `prelude/simple_op.rs` now has two new methods `extension_ref` and `init_signature`, so we can pass the `Weak<Extension>` around while the extension is being initialized and avoid deadlocks due to calling e.g. the `PRELUDE` lazy_static recursively. This caused all the simple op definitions to be updated with those defs. - `BOOL_T`, `QB_T`, `USIZE_T` are now (lowercase) methods. This is the main source of noise since it modifies a lot of tests. - `CustomType` in `types/custom.rs` has a new `extension_ref` field. - Running this revealed that one of the `hugr-model` roundtrip tests defined the `Array` type as coming from an inexistent `array` extension. I replaced it with `prelude`. --- drive-by: Make `just format` format non-default targets BREAKING CHANGE: Removed `CustomType::new_simple`. Custom types can no longer be const-constructed. BREAKING CHANGE: Added `init_signature` and `extension_ref` methods to the `MakeOpDef` trait. BREAKING CHANGE: Redefined the `const` types in the prelude to generator functions. --------- Co-authored-by: Mark Koch <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 3, 2024
When we do a serialization roundtrip on a hugr, its custom optypes are downgraded to `OpaqueOp`s and its `CustomType`s lose the weak link to their extension. This PR moves the pre-existing `OpaqueOp` resolution into a new `crate::extension::resolution` module and expands it to also update the custom type pointers. This can be run via the new `Hugr::resolve_extension_defs` method (currently called by `update_validate`). In addition, we accumulate the exact list of extensions required to define the hugr (this will be necessary for #1613). Note that this will probably be no longer necessary after we stabilize `hugr-model`, as we manually build the hugr when importing the model so the extensions should already be present at that point. In contrast to extension inference, this detects extensions needed to _define_ a hugr (vs runtime requirements). The inference one will be renamed as per #1734. A big chunk of this PR is fixing tests that used `finish_prelude_hugr` even though they required more extensions -.-' Look at the first commit for the actual changes. BREAKING CHANGE: Removed `resolve_opaque_op` and `resolve_extension_ops`. Use `Hugr::resolve_extension_defs` instead.
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 10, 2024
…1738) Closes #1613. Depends on #1739. Hugrs now keep an `Extensions` registry that is automatically computed when using the builder or deserializing from json (using the new `Hugr::load_json`). This set can contain unneeded extensions, but it should always be sufficient to validate the HUGR definition. Note that this is **not** runtime extensions (see #426, #1734). A big chunk of the diff is removing the extension registry when finishing building a hugr. The extension tracking is now done automatically while adding operations. drive-by: Remove unneeded `set_num_ports` call in `insert_hugr_internal`. BREAKING CHANGE: Removed `update_validate`. The hugr extensions should be resolved at load time, so we can use `validate` instead. BREAKING CHANGE: The builder `finish_hugr` function family no longer takes a registry as parameter, and the `_prelude` variants have been removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To allow validation of individual HUGRs with any root, without resorting to packages, which only currently allow modules. This would then mean packages would only need to contain modules (no extensions) and can be used mostly for linking. This also aligns well with the model work so should be done as a follow up to that. See #1433.
The text was updated successfully, but these errors were encountered: