-
Notifications
You must be signed in to change notification settings - Fork 18
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
Integrating BQSKit into MQTBench bench viewer (website) module #288
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
=======================================
- Coverage 93.5% 93.2% -0.4%
=======================================
Files 46 46
Lines 2416 2452 +36
=======================================
+ Hits 2260 2286 +26
- Misses 156 166 +10 ☔ View full report in Codecov by Sentry. |
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.
Hi @jagandecapri , I just have looked through this PR on a rather high level. In general, it looks good to me and I found only one copy paste error.
One thing I would like to change however is the sequence of the compilers. It should be Qiskit, TKET, BQSKit instead of the current order. In a similar fashion, the sequence of optimization level selections should be adjusted.
src/mqt/benchviewer/backend.py
Outdated
@@ -183,35 +188,69 @@ def filter_database(self, benchmark_config: BenchmarkConfiguration) -> list[str] | |||
| (self.database["benchmark"].isin(selected_nonscalable_benchmarks)) | |||
] | |||
|
|||
if benchmark_config.indep_qiskit_compiler: | |||
db_tmp1 = db_tmp.loc[(db_tmp["indep_flag"]) & (db_tmp["compiler"] == "qiskit")] | |||
if benchmark_config.indep_tket_compiler: |
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.
Here is the wrong compiler flag checked, must be indep_bqskit_compiler
instead of the tket one.
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.
Hi @nquetschlich, noted. I'll make the corrections.
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.
@nquetschlich I've changed the order of compilers and optimization level to make BQSKit last as below and fixed the bug on the incorrect bqskit flag check.
This PR addresses #287. BQSKit is added as the newest 🎉 compiler in MQTBench website. A screenshot of the updated website is as below. I’ve added BSQKit before Qiskit and TKET as I’m following alphabetical order of the compiler names.
The test cases are passing. However, BQSKit is not fully tested as no QASM file has been generated yet pending benchmark generation PR at #286 and the associated issues with BQSKit as described in the PR. I have added TODOs in the test cases to be fixed when BQSKit benchmark generation is working well.
This PR branches of from the
main
branch without the changes in #286. I kept the benchmark generation and benchmark viewer (website) branches separate for easier review. This PR should be merged after #286 works well.With that, looking forward to your kind review and discussions. 🙂