-
-
Notifications
You must be signed in to change notification settings - Fork 48
✨ Allow OpenQASM as mqt-cc input #1470
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
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds OpenQASM (.qasm) input handling to mqt-cc via a new QASM loader that parses QASM3 into QC MLIR, updates mqt-cc to link Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "mqt-cc CLI"
participant Router as "Input Router"
participant QASM as "QASM Importer"
participant MLIR as "MLIR Loader"
participant Pipeline as "Quantum Pipeline"
participant Writer as "Output Writer"
User->>CLI: invoke mqt-cc with args
CLI->>Router: parse options and filename
Router->>Router: check filename extension
alt filename ends with .qasm
Router->>QASM: loadQASMFile(filename, mlir::MLIRContext*)
QASM-->>Router: mlir::ModuleOp (QC)
else
Router->>MLIR: loadMLIRFile(filename, mlir::MLIRContext*)
MLIR-->>Router: mlir::ModuleOp
end
Router->>Pipeline: run quantum pipeline on module
Pipeline->>Writer: writeOutput(ModuleOp, filename)
Writer-->>User: compiled output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mlir/tools/mqt-cc/mqt-cc.cpp`:
- Around line 78-94: The loadQASMFile function currently redundantly opens the
file with mlir::openInputFile and does not handle exceptions from
qasm3::Importer::importf; remove the mlir::openInputFile check and instead call
qasm3::Importer::importf inside a try/catch block in loadQASMFile, catch
std::runtime_error and qasm3::CompilerError (and a generic std::exception
fallback), print clear error messages to errs() including the exception.what()
and filename, and return nullptr on error; on success continue to call
mlir::translateQuantumComputationToQC(context, qc).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mlir/tools/mqt-cc/mqt-cc.cpp`:
- Around line 50-52: The positional input help still reads "<input .mlir file>"
but with the cl::opt<bool> QASM_INPUT flag the tool can also accept OpenQASM
files; update the positional argument's help/description to be generic (e.g.
"<input file (MLIR or OpenQASM)>" or similar) so the CLI help isn't misleading.
Locate the positional input cl::opt/positional argument in mqt-cc.cpp and change
its desc string to a broader description that mentions MLIR and QASM (and
optionally give an example or extension list), leaving QASM_INPUT unchanged.
♻️ Duplicate comments (1)
mlir/tools/mqt-cc/mqt-cc.cpp (1)
78-87: Handle QASM import errors and stdin explicitly.
qasm3::Importer::importfcan throw on I/O or parse errors, and with--qasm-inputthe default filename is"-", which likely becomes a literal filename instead of stdin. This can crash the CLI or produce confusing output. Add explicit error handling and either support stdin (if the Importer has a stream overload) or fail fast with a clear message.🐛 Suggested fix (adjust stream API to actual Importer signature)
static mlir::OwningOpRef<mlir::ModuleOp> loadQASMFile(StringRef filename, mlir::MLIRContext* context) { - // Parse the input QASM using MQT-Core - const qc::QuantumComputation qc = qasm3::Importer::importf(filename.str()); - // Translate to MLIR dialect QC - return mlir::translateQuantumComputationToQC(context, qc); + try { + if (filename == "-") { + errs() << "stdin is not supported for --qasm-input yet; pass a .qasm file path.\n"; + return nullptr; + // If available, prefer a stream-based import overload instead: + // auto qc = qasm3::Importer::import(std::cin); + // return mlir::translateQuantumComputationToQC(context, qc); + } + const qc::QuantumComputation qc = qasm3::Importer::importf(filename.str()); + return mlir::translateQuantumComputationToQC(context, qc); + } catch (const qasm3::CompilerError& e) { + errs() << "Failed to parse QASM file '" << filename << "': " << e.what() + << "\n"; + return nullptr; + } catch (const std::exception& e) { + errs() << "Failed to load QASM file '" << filename << "': " << e.what() + << "\n"; + return nullptr; + } }To verify the available Importer overloads and exception types, you can run:
#!/bin/bash # Check Importer API and exception types fd -a -e hpp -e h 'Importer' | while read -r f; do echo "== $f =="; rg -n 'importf\s*\(|import\s*\(.*istream|CompilerError|throw' "$f"; doneExpected results: confirm whether a stream-based
import(std::istream&)exists and which exception types are documented/thrown.Also applies to: 149-154
burgholzer
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.
This is great and a welcome addition. Some nitpicks in the comments. This should not be long until it can go in.
You might want to add the PR number to the change log.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mlir/tools/mqt-cc/mqt-cc.cpp`:
- Around line 154-162: The code prints a duplicate error because
loadQASMFile/loadMLIRFile already emit detailed errors before returning nullptr;
remove the centralized generic message in main by deleting the errs() << "Failed
to load input file: " << INPUT_FILENAME << "\n"; check the nullptr check that
uses module (after calls to loadQASMFile/loadMLIRFile) remains to return 1 but
does not re-log the error; references: INPUT_FILENAME, loadQASMFile,
loadMLIRFile, module, errs().
4cb5046 to
d8f8e86
Compare
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mlir/tools/mqt-cc/mqt-cc.cpp`:
- Around line 158-165: Update the file-extension check so
INPUT_FILENAME.getValue().ends_with(".qasm") also matches ".qasm2" and ".qasm3"
and route those to loadQASMFile; specifically modify the conditional that
chooses between loadQASMFile and loadMLIRFile (the branch using
INPUT_FILENAME.getValue().ends_with) to accept ".qasm", ".qasm2", or ".qasm3"
(and optionally normalize case if desired) so module is set by loadQASMFile for
any of those extensions.
burgholzer
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.
One last round of feedback here.
Co-authored-by: Lukas Burgholzer <[email protected]> Signed-off-by: Tamino Bauknecht <[email protected]>
Signed-off-by: burgholzer <[email protected]>
burgholzer
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.
Let's go 🚀 Let's see which of the PRs makes it in first #1426, the other one will need to have its conflicts on the changelog resolved.
Description
For the evaluation of my thesis, I will need to compile OpenQASM files using optimizations in the
QCOdialect.The easiest solution was for me to directly add this feature to the
mqt-cctool.Since I think this might be a valuable addition in general, I wanted to propose to merge this feature to main.
Additionally, I added proper handling of the CLI argument parsing since currently errors are just ignored without any user output.
Checklist: