-
Notifications
You must be signed in to change notification settings - Fork 35
🎨 Refactor NAComputation
with concrete base classes for every operation and ouput new .naviz
format
#846
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
ebe8ff1
to
ba95268
Compare
mqt-naviz'
formatmqt-naviz
format
7c681ce
to
4edea4a
Compare
@burgholzer The tests should run through now and all your feedback should be integrated to the best of my knowledge. |
Great! Many thanks for addressing this so quickly 🙏🏼 |
@burgholzer There is (at least) one issue remaining. I used the convention to append a |
Hm. Probably the cleanest solution would be to adjust the clang-tidy config so that this is not a violation anymore. |
Apparently, there is a specific key for that option. See the changes from the last commit. |
I am pretty sure that is not what we want, as this enforces the trailing underscore. And I am also pretty sure you do not have the time resources to go through all of MQT Core and change that. |
mqt-naviz
formatNAComputation
with concrete base classes for every operation and ouput new .naviz
format
…lation (#849) ## Description This pull request adds support for handling empty quantum and classical registers during the translation process from Qiskit to MQT. This update ensures proper translation and eliminates any potential errors caused by empty registers. This has come up while working on munich-quantum-toolkit/ddsim#336 ## Checklist: - [x] The pull request only contains commits that are related to it. - [x] I have added appropriate tests and documentation. - [x] I have made sure that all CI jobs on GitHub pass. - [x] The pull request introduces no new warnings and follows the project's style guidelines. Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
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.
Many thanks for working through all the comments.
This is a really clean PR. I like that very much 😌
I just put on some finishing touches in the last couple of commits. Specifically:
- removed two redundant files that were left over from before the last iteration
- added
@file
docstrings to all new files. (important for getting proper doxygen output) - completed some missing docstrings
- shortened some existing ones where a one-line brief was sufficient
- adjusted the spacing in headers so that the files don't look so cluttered.
- applied some left-over suggestions in the NA computation.
Once CI is green, I will go ahead and merge this.
I'll also create a new 3.0 Beta release so that it becomes easier to directly rely on this in QMAP.
Description
The tool
mqt-naviz
purposefully introduces a new textual format that allows to execute operations at a specific time stamp. The current output format of NA computation is not really compatible even thoughmqt-naviz
supports a legacy feature to read in this old format with some caveats.This issue triggered a wider refactoring of the
NAComputation
. Its entire structure is refactored. Most importantly, concrete Operation have their own class representation inheriting from some super class. At the end, all operations inherit from the classOp
. As part of the refactoring, there is a straight forward way to test whether an operation is of some particular type and then it can be casted to this type.Checklist: