♻️ Refactor IR Verification Logic#1781
Conversation
…ch-quantum-toolkit/core into enh/ir-verifier-refactoring
…ch-quantum-toolkit/core into enh/ir-verifier-refactoring
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Signed-off-by: burgholzer <burgholzer@me.com>
burgholzer
left a comment
There was a problem hiding this comment.
Thanks @MatthiasReumann for working on this! 🙏🏼 much appreciated.
To be honest, I did not fully think through all of the verifier rewrites, which are quite substantial. But they do look convincing.
I just pushed some fairly minor cosmetic changes (things that CLion showed as warnings).
Other than that, I have two smaller comments inline and one bigger one here:
It is nice that the existing test suite is still running. However, that was the status quo also before this PR.
To gain some certainty that this addresses the underlying problems we have been trying to solve, it would be helpful to add at least one new test case that would have failed before this PR.
|
@burgholzer Thanks for the review! 🙏
I've added an additional test to the |
burgholzer
left a comment
There was a problem hiding this comment.
LGTM modulo one typo.
Feel free to resolve and merge after CI is green.
Co-authored-by: Lukas Burgholzer <burgholzer@me.com> Signed-off-by: matthias <matthias@bereumann.com>
Description
Among other things, resolves #1752. The goal of this pull request is to simplify the verifier, while improving its performance, and make it somewhat bulletproof by explicitly stating its assumptions.
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.