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.
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
Rewrite Extension section for type info #101
Rewrite Extension section for type info #101
Changes from 8 commits
69540f5
8b58246
1b50c25
166c219
74ac201
0268a68
78c0411
4ddc925
a7c62e9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It might be worthwhile, but not in it's current form. The key point (imo) is that we're shelling out to some custom rust-implemented method and, because we know the type signature, we know how we should interpret the things that we get back. I think this is true of all 3 kinds of operations?
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.
Yes, I think so - key point vs. the previous text is that how the operation type is computed (2 ways above) is independent of how (tools figure out) the function of the operation (including whether or not an implementation of
try_lower
, or a Hugr, is provided)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.
But I think the language about downcasting is rather implementation-specific (e.g. in Rust would we use
Any
anddowncast_ref
??) so probably doesn't belong in this docThere 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.
@croyzor I'm a bit unclear how this works. We've imported Quantum, and we say
Q
is defined there - but, as an alias? Or anOpaque
? Those are, AFAI am aware, the only two ways for an extension to define new types. Maybe alias works, or maybeQ
is not a good example.Elsewhere (below) I've been writing
Opaque(complex_matrix,...)
but maybe that should just becomplex_matrix
?? and that should be imported from another resource - in which case, lets do a qualified import for that....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.
I think the resource requirements should go back in. We can have operations within certain resources which require other resources. E.g. the
MatMul
example could have an extra requirement on someLinAlg
resourceThere 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.
I move that this requirement is either (a) use of types defined in
LinAlg
, i.e. thecomplex_matrix
type mentioned in this comment; or (b) a property of the implementation, i.e. the rust/binary code (for what sets of resourcestry_lower
will succeed or fail), rather than the interface.That is, at present the YAML does not specify when
try_lower
will succeed or fail.