-
Notifications
You must be signed in to change notification settings - Fork 1
TerminalLogicalMeasurement Validation #641
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
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
david-pl
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.
Looks good, but I'm not sure about some stuff (see comments).
weinbe58
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.
overall looks good 🚀 just a few comments to address
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.
To fix the circular import put this validation inside logical folder since this is only a logical program validation.
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.
By extension that would also mean that the current logical_validation should also go in there as well? Just as a heads up I'll be splitting up the rules in logical_validation right after this PR (so the scf, gate, and func impls are separate validations you can compose)
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.
Gave it a shot, didn't play nice ): Willing to revisit in a subsequent PR though, that will involve shuffling more things around anyways
…l options for definition
weinbe58
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
This is what I think should be sufficient for TerminalLogicalMeasurement Validation.
I enforce the following conditions:
There are some additions that come to mind - ranging from immediately relevant to some things that are further out in terms of infrastructure: