-
-
Notifications
You must be signed in to change notification settings - Fork 162
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
Include calculated error in calibrator logs #2038
Conversation
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.
Hello @kozhukalov, thank you for submitting a PR to Mitiq! We will respond as soon as possible, and if you have any questions in the meantime, you can ask us on the Unitary Fund Discord.
9564696
to
467b489
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2038 +/- ##
=======================================
Coverage 98.30% 98.30%
=======================================
Files 87 87
Lines 4118 4125 +7
=======================================
+ Hits 4048 4055 +7
Misses 70 70
☔ View full report in Codecov by Sentry. |
2c46072
to
221924d
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.
Really nice work Vladimir! Thanks for adding this!
I think we should add the errors by default as part of the table instead of adding the additional log_errors
option. IMO if someone is asking for logs, lets provide everything!
Ok, I'll make it default. The only concern here is backward compatibility. Some people probably try to parse the performance log with their automation tools and we can break their pipelines. However I am new in Mitiq. Not sure how important it is. |
31f798c
to
e44cbea
Compare
3cafa4a
to
617a3aa
Compare
I am not sure what is wrong with the docs/readthedocs.org:mitiq check. It says |
@kozhukalov This is a bug in RTD. When there are multiple PRs running checks at the same time, RTD fails to build the documentation in all of them. I do not know a way to re-trigger this build besides making a dummy commit. |
@purva-thakre The issue is that even if this RTD job is green it the becomes red later when another PR runs RTD job. So I think it should not be voting. |
Calibration log table is extended with errors calculated during calibration and sorted so that best techniques are on the top of the table. Fixes unitaryfund#2014
@kozhukalov Yes, I have noticed that the green check becomes red later. Possibly some bug in RTD. Don't worry too much about the RTD build failure as long as you are not making changes to the documentation. |
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.
Awesome work with this! Thanks for the nice simple upgrade.
Description
Calibration log table is extended with errors calculated
during calibration and sorted so that best
techniques are on the top of the table.
Fixes #2014
License
Before opening the PR, please ensure you have completed the following where appropriate.