-
Notifications
You must be signed in to change notification settings - Fork 16
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
MQTPredictor update #299
MQTPredictor update #299
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #299 +/- ##
=======================================
- Coverage 93.2% 91.9% -1.3%
=======================================
Files 46 46
Lines 2407 2427 +20
=======================================
- Hits 2244 2232 -12
- Misses 163 195 +32 ☔ 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.
Thanks a lot for those changes. I just have a few questions to better understand them.
Additionally, I have one larger comment that probably requires a moderate effort: We should substitute the M3 for the M2 instead of keeping both devices. This means, that all files must be clear of the M2 device including the configuration file.
For the benchviewer, we currently stick to the M2 device and, therefore, those files must not be adjusted. The rationale is, that we will soon update to Qiskit v1.0.0 and then re-create all benchmark files and do not want to do it now additionally since it comes with a significant effort.
resolves #279 |
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.
As soon as the code comments are clarified and the currently failing test is fixed, this PR is ready to be merged.
Mostly bug fixes that became clear when trying to update MQTPredictor based on the new device class.