-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: supporting encoding const-parametrized ops #80
Conversation
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.
Awesome!
src/circuit/command.rs
Outdated
optype | ||
.static_input() | ||
.map(|_| PortOffset::new_incoming(sig.input.len()).into()), |
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.
Ideally we shouldn't need to access portgraph primitives, nor should we set here what's the offset of the static input.
I can add an issue in hugr to implement an OpType::static_input_port
(similar to OpType::other_port
).
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, sounds good
|
||
// TODO: Update op.params. Leave untouched the ones that contain free variables. | ||
let mut op: circuit_json::Operation = op.into_operation(); | ||
if !params.is_empty() { |
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.
With this we recover all parameters that have been encoded as inputs. It should be easy to support transparent pass-through of parameters we don't understand yet (e.g. x+1
) and retrieve them from the metadata here.
This is OK for now, but I'll add a followup issue.
Co-authored-by: Agustín Borgna <[email protected]>
just use 0 directly and add comment
Also refactor json tests to use fixtures (easier to add new ones)
Check some command equality in those tests for circuits with matching command ordering (could parametrise the comparison function in future to skip this)