-
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
feat: Special cased array, float and int constants in hugr-model export #1857
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1857 +/- ##
==========================================
- Coverage 86.52% 86.52% -0.01%
==========================================
Files 194 194
Lines 35250 35352 +102
Branches 32063 32165 +102
==========================================
+ Hits 30501 30588 +87
- Misses 2973 2975 +2
- Partials 1776 1789 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary
|
7b7bc0d
to
b588731
Compare
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.
🚀
(@ arithmetic.float.types.float64)]])] | ||
(ext)))))) | ||
|
||
(define-func example.f64 | ||
(define-func example.f64-json |
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.
is this still using json?
@@ -27,12 +27,13 @@ declarative = ["hugr-core/declarative"] | |||
model_unstable = ["hugr-core/model_unstable", "hugr-model"] | |||
llvm = ["hugr-llvm/llvm14-0"] | |||
llvm-test = ["hugr-llvm/llvm14-0", "hugr-llvm/test-utils"] | |||
default = ["model_unstable"] |
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.
😓
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.
Uhm. I think we should do unstable XOR default
self.print_text(format!("{:?}", value.into_inner())); | ||
Ok(()) | ||
} | ||
Term::FloatType => { |
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.
looks like float type isn't being covered, pls add to test
This PR special cases the import/export of array, float and int constants so that they are represented as terms in
hugr-model
. This representation is a lot more efficient to handle than the encoding as JSON. In the future we should do this for all constants.