feat: Export the core data structures to a model representation #1414

zrho · 2024-08-12T11:42:47Z

This PR introduces a draft for the hugr-model data structures together with a function to export a Hugr into this format.

codecov · 2024-08-15T11:13:18Z

Codecov Report

Attention: Patch coverage is 0% with 266 lines in your changes missing coverage. Please review.

Project coverage is 86.44%. Comparing base (67f0668) to head (2d4b341).

Files	Patch %	Lines
hugr-core/src/export.rs	0.00%	266 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1414      +/-   ##
==========================================
- Coverage   87.55%   86.44%   -1.11%     
==========================================
  Files         119      120       +1     
  Lines       20781    21047     +266     
  Branches    18053    18319     +266     
==========================================
  Hits        18194    18194              
- Misses       1802     2068     +266     
  Partials      785      785

Flag	Coverage Δ
python	`90.61% <ø> (ø)`
rust	`85.82% <0.00%> (-1.27%)`	⬇️

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.

acl-cqc

Thanks, Lukas - a few thoughts.

Presumably if we go with a system of "constraints", in order to express the expressible parts of binary compute_signature, we'll also need to use the hugr-model for serializaiton of extension sets...

acl-cqc · 2024-08-19T12:47:46Z

hugr-core/src/export.rs

+    hugr: &'a Hugr,
+    /// The module that is being built.
+    module: model::Module,
+    /// Mapping from ports to edge indices.


This makes sense after I've looked at get_edge_id but a bit more explanation here would help: this is where people will look for docs first!

hugr-core/src/export.rs

acl-cqc · 2024-08-19T12:57:46Z

hugr-core/src/export.rs

+            TypeArg::BoundedNat { n } => ctx.make_term(model::Term::Nat(*n)),
+            TypeArg::String { arg } => ctx.make_term(model::Term::Str(arg.into())),
+            TypeArg::Sequence { elems } => {
+                // For now we assume that the sequence is meant to be a list.


Ok, yah this is the tricky bit. I admit that none of the options seem great here.

Add an ExtensionRegistry to the Context. This doesn't seem to bad, but (although I think necessary) is not sufficient - you also need to pass a TypeParam around, with recursive calls passing an inner TypeParam obtained by destructuring the outer one (panic if TypeParam and TypeArg doesn't correspond, as this should have been detected by validate). Two possible ways:

add a TypeParam to the Context, or

(I suspect better) define a new trait export-with-TypeParam

Change TypeArg to distinguish between List and Tuple

Change the model not to distinguish between List and Tuple.

(1.) works, has least fallout elsewhere, but is the greatest "yeuch" here. (2.) is most disruptive to existing code. I'll leave you to think about (3.)...

One could argue that if we aren't prepared to make such awkward format conversions as (1.), then the model has failed in its primary purpose of decoupling serialization format from core/Rust-struct representation

acl-cqc · 2024-08-19T13:03:33Z

hugr-core/src/types.rs

acl-cqc · 2024-08-19T13:06:32Z

hugr-model/src/lib.rs

@@ -0,0 +1,5 @@
+//! The data model of the HUGR intermediate representation.
+//! This crate defines data structures that capture the structure of a HUGR graph and
+//! all its associated information in a form that can be stored on disk. The data structures


nit: does anyone still use "disk"s nowadays? :)

perhaps something like "in a form designed for simple and efficient serialization, rather than traversal or modification"??? Or we could say this is for serialization, storage and interchange?

I think it's used metaphorically, like the floppy disk symbol meaning "save" although it's most likely not being saved on a floppy disk.

acl-cqc · 2024-08-19T13:18:02Z

hugr-model/src/v0/mod.rs

+        /// The name of the term.
+        pub name: Symbol,
+        /// Arguments to the term.
+        pub args: TinyVec<[TermId; 3]>,


Similar comment about triples

acl-cqc · 2024-08-19T13:19:09Z

hugr-core/src/export.rs

+    }
+
+    pub fn make_term(&mut self, term: model::Term) -> model::TermId {
+        let index = self.module.terms.len();


Is this the place to do hash-consing?? (I.e. look for an existing Term that matches the suppiled, and if so then return that TermId rather than pushing)

It will be useful to associate ids with uses, i.e be able to trace problems back to the source by figuring out which instance of a term we're looking at. I'm looking in to hashconsing at serialisation time, and preserving all the ids that point to the same term. We can apply this to the model separately without changing the model behaviour I think.

have experimented with doing hashconsing here: e4ef7be
I think it's worthwhile.

acl-cqc · 2024-08-19T13:22:38Z

hugr-model/src/v0/mod.rs

+//!
+//! # Terms
+//!
+//! Terms form a meta language that is used to describe types, parameters and metadata that


This is a design point that has probably been addressed elsewhere, but do we really want structured metadata like this?

Ok, I can see many use cases for metadata which does contain....Hugr-defined things. But not necessarily Terms - maybe Hugrs or NodeId's or other things. And, how is unstructured metadata represented? (I see that a MetaItem contains exactly one Term, I haven't seen the alternative - am I looking in the wrong place?)

@zrho made the point that if what we want is unstructured json you can just use the "string" metadata type always. I personally favour structure only enough structure in metadata to allow for reserved key names we can check for, and fully unstructured payloads.

If you want unstructured metadata, you can still put it into a string. Structured metadata allows validation via typechecking.

Ah OK yes String is good, right

acl-cqc · 2024-08-19T13:24:41Z

hugr-model/src/v0/mod.rs

+
+    /// The type of the port.
+    ///
+    /// This must be a term of type `Type`.


Also, will be the same for all Ports with the same edge

acl-cqc · 2024-08-19T13:28:43Z

hugr-model/src/v0/mod.rs

+    /// Constraints on the parameters of the scheme.
+    ///
+    /// All parameters are available as variables within the constraints.
+    /// The constraints must be terms of type `Constraint`.


Right - I'm puzzled here. I don't see any enum Term variants that have type Constraint; I see only one for the type of Constraints, and I can't understand how that would be used. Am I missing something, looking at this the wrong way, or is this just WIP?

This is all a big WIP.

aborgna-q

Some housekeeping needed when adding a new package:

Add a README. Just copy the template from the other crates.
Add this to release-plz.toml. We'll still need to do some manual deployment before the next hugr-core release, as release-plz does not have permission to create new packages.
```
[[package]]
name = "hugr-cli"
release = true
```
Also add it to the changelog_include for hugr
Add the new directory it to .github/change-filters.yml

aborgna-q · 2024-08-19T16:08:41Z

hugr-model/Cargo.toml

+edition.workspace = true
+homepage.workspace = true
+repository.workspace = true
+license.workspace = true


Suggested change

license.workspace = true

license.workspace = true

readme = "README.md"

documentation = "https://docs.rs/hugr-model/"

description = "Serializable model for Quantinuum's HUGR"

keywords = ["Quantum", "Quantinuum"]

categories = ["compilers"]

zrho force-pushed the zrho/model branch from ee0379a to ca60314 Compare August 12, 2024 16:08

zrho changed the title ~~Draft: Export to text format~~ feat: Export to text format Aug 12, 2024

zrho changed the title ~~feat: Export to text format~~ feat: Export the core data structures to a model representation Aug 15, 2024

zrho force-pushed the zrho/model branch from 714583e to 86ec989 Compare August 15, 2024 10:53

hugr-model draft and export from hugr-core.

01bfc13

zrho force-pushed the zrho/model branch from 86ec989 to 01bfc13 Compare August 15, 2024 11:08

ss2165 mentioned this pull request Aug 15, 2024

HUGR Model and binary serialisation format. #1433

Open

13 tasks

zrho and others added 4 commits August 15, 2024 15:34

More docs and the Constraint term.

4c71a2e

More documentation in model. Also added PartialEq, Eq, Hash derives.

6f2a67e

fix: incorrectly using terms length for node (#1443)

53c6826

Edge representation in model: make ports refer to the id of the edge.

4d9e503

zrho force-pushed the zrho/model branch from bb944f1 to 4d9e503 Compare August 19, 2024 12:27

acl-cqc reviewed Aug 19, 2024

View reviewed changes

aborgna-q reviewed Aug 19, 2024

View reviewed changes

ss2165 force-pushed the zrho/model branch from 2d4b341 to 4d9e503 Compare August 20, 2024 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Export the core data structures to a model representation #1414

feat: Export the core data structures to a model representation #1414

zrho commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 15, 2024 •

edited

Loading

acl-cqc left a comment

acl-cqc Aug 19, 2024

acl-cqc Aug 19, 2024

acl-cqc Aug 19, 2024

acl-cqc Aug 19, 2024

zrho Aug 19, 2024

acl-cqc Aug 19, 2024

acl-cqc Aug 19, 2024

ss2165 Aug 19, 2024

ss2165 Aug 20, 2024

acl-cqc Aug 19, 2024

ss2165 Aug 19, 2024

zrho Aug 19, 2024

acl-cqc Aug 20, 2024

acl-cqc Aug 19, 2024

acl-cqc Aug 19, 2024

zrho Aug 19, 2024

aborgna-q left a comment

aborgna-q Aug 19, 2024

-license.workspace = true
+license.workspace = true
+readme = "README.md"
+documentation = "https://docs.rs/hugr-model/"
+description = "Serializable model for Quantinuum's HUGR"
+keywords = ["Quantum", "Quantinuum"]
+categories = ["compilers"]

feat: Export the core data structures to a model representation #1414

Are you sure you want to change the base?

feat: Export the core data structures to a model representation #1414

Conversation

zrho commented Aug 12, 2024 • edited Loading

codecov bot commented Aug 15, 2024 • edited Loading

Codecov Report

acl-cqc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aborgna-q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zrho commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 15, 2024 •

edited

Loading