-
Notifications
You must be signed in to change notification settings - Fork 11
feat: Add CopyableExpressionAST #1209
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
Conversation
3249a88 to
16e358a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1209 +/- ##
==========================================
+ Coverage 78.77% 78.97% +0.20%
==========================================
Files 153 154 +1
Lines 18910 19111 +201
Branches 17808 18009 +201
==========================================
+ Hits 14896 15093 +197
- Misses 3070 3076 +6
+ Partials 944 942 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cadeb13 to
244a2f8
Compare
244a2f8 to
0ba8534
Compare
acl-cqc
left a comment
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've suggested a lot of renamings and minor stuff, but nothing major, so I guess I'm happy for this to go in, but with the proviso...
...that I am still not certain whether we shouldn't have a single SiblingSubgraph for all the classical computation associated with a set of intervals rather than many potentially-overlapping ones...maybe I would need to see some examples
I agree that the fact that multiple copyable expressions might have overlapping nodes is not great. That being said
Overall, I agree that there are further follow-ups and clean-ups that should be considered and executed, but this is a step in the right direction: it's helping construct a layer of abstraction above |
|
Thanks a lot Alan for the review!! Will now go through your comments and merge this in. |
Co-authored-by: Alan Lawrence <[email protected]>
Co-authored-by: Alan Lawrence <[email protected]>
Co-authored-by: Alan Lawrence <[email protected]>
I want to define a
Subcircuitas a combination ofIntervals on resource paths (the parts with linear, non-copyable data), and ASTs of fully classical, copyable data.IntervalCopyableExpressionASTtype that corresponds to ASTs of classical data in hugr graphs.