-
Notifications
You must be signed in to change notification settings - Fork 1
Remove op and wire dialects #548
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
|
|
||
|
|
||
| @wire.dialect.register | ||
| @qubit.dialect.register |
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.
@johnzl-777 I'm not sure if this is correct. Do we even still need the WrapAnalysis?
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 WrapOpSiteAnalysis isn't necessary anymore but the WrapAddressAnalysis is. WrapAnalysis just lets me store analysis results in the statement hints so that other rewrite rules can access them (for example, in SquinNoiseToStim I query the hints for address analysis data).
I think one could get away without wrapping altogether? The cost is that I have to manually feed the analysis results to the rules that need the data
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.
Okay, thanks for clarifying. Then I'm leaving this as is. The site analysis is already gone altogether, since we don't have Op anymore, it doesn't make any sense.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
johnzl-777
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.
LGTM so far!
|
Do not have this blocked #508. That PR will be simplfied a bit after this is merged. I can take care of merging any changes. |
johnzl-777
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.
Farewell wire and op, may your contributions to bloqade-circuit never be forgotten ❤️
This is the final step of the refactor in #492. We need to merge #508 first, however.