-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add custom serde (de)serialization for compact hugr #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ss2165
force-pushed
the
feature/new-module
branch
from
April 5, 2023 13:04
69e90d7
to
7ced3f2
Compare
ss2165
force-pushed
the
feature/serialize
branch
from
April 5, 2023 13:07
c68d034
to
6088e3a
Compare
ss2165
force-pushed
the
feature/new-module
branch
from
April 11, 2023 15:43
7ced3f2
to
53b1a23
Compare
ss2165
force-pushed
the
feature/serialize
branch
from
April 12, 2023 09:44
6088e3a
to
4a00e51
Compare
aborgna-q
approved these changes
Apr 12, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ss2165
force-pushed
the
feature/new-module
branch
from
April 13, 2023 09:45
e9eeafb
to
0508cc0
Compare
ss2165
force-pushed
the
feature/serialize
branch
from
April 13, 2023 09:51
4597cf4
to
4551ca0
Compare
ss2165
force-pushed
the
feature/serialize
branch
from
April 13, 2023 10:05
b52693d
to
23bae83
Compare
aborgna-q
approved these changes
Apr 13, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 17, 2024
Updates the `Display` implementation of poly functions, functions, types, type arguments, and type params so its easier to read. Comparison below. I added the newlines manually. Before: ```swift forall Any _0.map(|i|i.to_string()).unwrap_or("-".to_string()). [array([ Variable { v: TypeArgVariable { idx: 1, cached_decl: BoundedNat { bound: UpperBound(None) } } }, Type { ty: TypeBase(Sum(General { rows: [TypeRowBase { types: [] }, TypeRowBase { types: [TypeBase(Variable(0, Any), Any)] }] }), Any) } ])] -> [[]][[ array([ Variable { v: TypeArgVariable { idx: 1, cached_decl: BoundedNat { bound: UpperBound(None) } } }, Type { ty: TypeBase(Sum(General { rows: [TypeRowBase { types: [] }, TypeRowBase { types: [TypeBase(Variable(0, Any), Any)] }] }), Any) } ]), int([BoundedNat { n: 6 }]) ]] ``` (yes, that is some hardcoded rust source) After: ```swift ∀ (var#0 : Type) (var#1 : Nat). [array(var#1, Type([]+[var#0]))] -> [[array(var#1, Type([]+[var#0])), int(6)]] ``` That is, a generic function on a type var and an unbounded natural that takes an array of (nat, option type) and returns a tuple with the array and an int. It may be better if the tuple display used `()` instead of `[]`, but that can be discussed later. Update after review suggestions: ```swift ∀ (#0 : Type) (#1 : Nat). [array(#1, []+[#0])] -> [[array(#1, []+[#0]), int(6)]] ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Serialization implemented via an intermediate struct which maps
(graph, hierarchy)
to(nodes, edges)
as this is a more natural encoding, and more useful for front-ends.A key limitation of this is that it requires nodes to be compacted beforehand, so serialisation would invalidate node indices. In most use cases though, this should be sufficient.
If more direct serialisation of the datastructures is desirable in future (e.g. for storing intermediate data to disk), we can return to using serde derive directives directly, and expose the intermediate
SerHugr
structure (requiring explicit conversion in and out of this for "ergonomic" serialised format).If that is not required, we can instead implement
Serialize
andDeserialize
directly without going via the intermediate.The current state is left as a reasonable performance compromise which leaves both future options open.