♻️ Own neutral-atom OpenQASM serialization - #1124
Conversation
f5750a8 to
ede4c94
Compare
denialhaag
left a comment
There was a problem hiding this comment.
Thanks for also updating the implementation here, @simon1hofmann! 😎
Depending on how much we care about how clean the implementation is here, this could trigger some additional work in MQT Core. Otherwise, the OpenQASM-related changes largely look good to me. I'll try to get #1111 as soon as possible so that this PR isn't unnecessarily blocked.
Unrelated to that, I'm surprised to see all the MLIR-related changes in this PR. I don't think this should be necessary. 🤔
There was a problem hiding this comment.
I guess this is related to my previous comment, but I hope that we can do without setting up MLIR when v4 is released. 🤔
There was a problem hiding this comment.
@burgholzer, I lost track of how we want to handle this when v4 is released. Will all of the downstream repositories need MLIR as well? 🤔
There was a problem hiding this comment.
🤖 AI text below 🤖
Agreed. The temporary MLIR workflow and packaging setup has been removed from the rewritten branch. This PR is intentionally a draft with expected CI failures until Core v4 is available.
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
354dd87 to
a2478c8
Compare
Cpp-Linter Report
|
There was a problem hiding this comment.
This LGTM now as well, and I'll leave the final say to @burgholzer! 😎
Edit: Well, I won't request a review from @burgholzer just yet. This will probably stay open for some weeks.
Signed-off-by: Daniel Haag <121057143+denialhaag@users.noreply.github.com>
denialhaag
left a comment
There was a problem hiding this comment.
Ahh, looks like the build is currently failing. 🤔
burgholzer
left a comment
There was a problem hiding this comment.
This one was small enough to quickly browse through. Besides the CI errors flagged by Daniel, this looks pretty good to me.
Will keep failing until MQT Core v4 is released due to the missing header. |
That makes a lot of sense. I could have figured that out myself. 🧍 |
🤖 AI text below 🤖
Description
Adapt the neutral-atom extraction in #1111 to the dedicated serializer from
MQT Core #2249.
operations;
custom leaf operations;
DataLogger;This is a stacked pull request targeting the
na-transferbranch.Draft status
This PR intentionally remains a draft until MQT Core v4 is released. It does
not pin an intermediate Core commit and does not add temporary MLIR/CI setup.
CI is therefore expected to fail against the currently released Core version.
Dependencies
and the corresponding MQT Core v4 release.
Validation
Validated against the Core PR branch before removing the temporary pin:
uvx nox -s lintAI assistance
Codex materially assisted with implementation, tests, integration validation,
and this pull request description. A human must review and understand the
changes before marking this pull request ready.
Checklist
If PR contains AI-assisted content:
🤖 *AI text below* 🤖(titles are exempt).