-
-
Notifications
You must be signed in to change notification settings - Fork 6
Add Error Budget Optimization Approach #155
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #155 +/- ##
=====================================
Coverage 84.0% 84.0%
=====================================
Files 8 8
Lines 883 883
=====================================
Hits 742 742
Misses 141 141
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thanks for the PR @tobi-forster 👌🏻
I just went through the code here and left a couple of comments inline.
There are three things I am slightly concerned about:
- this is not tested in any way so that we could be sure it actually works and continues to work over time. This also directly ties into the fact that this is not packaged into the mqt-problemsolver Python package and its structure, which means that everyone will have to figure out themselves how to actually run this and how to get all the necessary dependencies. That's not really too user friendly. People also cannot simply do a "pip install mqt-problemsolver" to try this out.
- neither mypy nor ruff apparently run over these files at the moment. If they would, they would highlight quite some things. Most obviously the missing static typing information. I'd argue both checkers should be instructed to run over the added files.
- do we really need to store the training results in the repository? How long does generating that data take?
Co-authored-by: Lukas Burgholzer <burgholzer@me.com> Signed-off-by: Tobias Forster <158472669+tobi-forster@users.noreply.github.com>
Co-authored-by: Lukas Burgholzer <burgholzer@me.com> Signed-off-by: Tobias Forster <158472669+tobi-forster@users.noreply.github.com>
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.
Thanks for your additional work here.
For a first start, this should almost be ready to merge. One more round of comments 🙂
resource_estimation/Error_Budget_Optimization/example_use.ipynb
Outdated
Show resolved
Hide resolved
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
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.
I just spent a couple of hours going through the changes here and pushed the results of that in the last couple of commits.
First of all, I simplified some of the configuration changes so that only necessary things are changed.
Secondly, I tried to incorporate minor code quality improvements.
Lastly, I tried to refactor the data generation code so that it does not load all the circuits into memory and instead streams the data.
I have one big issue with this PR: I tried for way longer that I would habe like to to actually run the Jupyter notebook. However, no matter which combination of dependency versions and/or zip file data or generated benchmarks I tried, it failed. I could not get past the data generation.
Most of the time, the errors were of the nature
thread '<unnamed>' panicked at crates/accelerate/src/elide_permutations.rs:73:25:
internal error: entered unreachable code
when using the MQT Bench generated circuits.
Or, even more often
Error processing circuit dj: Failed to export QASM3 source.
I am slightly worried that this PR is currently pinning qsharp to 1.9, while 1.19 has already been released. That's 10 minor versions of changes that might have fixed or improved things.
Anyhow, I cannot get this to run and I don't have more time at the moment. Maybe you have more success investigating.
…list of logical counts used in the paper
…d local variable Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Tobias Forster <158472669+tobi-forster@users.noreply.github.com>
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 roll with this for now 🚀
The last couple of iterations definitely improved the implementation here.
Let's tackle the tighter integration into the existing package in a follow-up PR.
I'll just add some labels to this PR. Feel free to merge afterwards.
This PR adds the approach to optimize the error budget distribution for a given quantum circuit based on a machine learning model. The source code can be found in the three .py files under resource_estimation/Error_Budget_Optimization, while an example is provided in the example_use.ipynb notebook.