-
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
🎨 Improved SupermarQ feature calculation #298
Conversation
…tion and copied/improved for two out of five features
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
=======================================
- Coverage 93.6% 93.3% -0.3%
=======================================
Files 46 46
Lines 2416 2405 -11
=======================================
- Hits 2262 2245 -17
- Misses 154 160 +6 ☔ 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.
I really like the changes here! Makes the code quite a bit cleaner.
I believe this can be taken one step further though as indicated in the inline comments.
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
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.
Many thanks for the changes. Looking really good already. I just have one suggestion for the code itself and one for the tests.
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
…QTBench into improved_supermarq_calculation
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.
Great. Thanks for all the extra effort. 🎉
I compared our implementation of the SupermarQ feature calculation with their implementation and copied their code for
liveness
andprogram_communication
because it seemed better to me in terms of understadability.