Skip to content
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

Adding error message when fitting failed #745

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Mar 4, 2024

Closes #743 .

The following implementation does not require a marker file which I think is better.

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.
  • Compatibility with Qibo modules (Please edit this section if the current pull request is not compatible with the following branches).
    • Qibo: master
    • Qibolab: main
    • Qibolab_platforms_qrc: main

@andrea-pasquale andrea-pasquale added the enhancement New feature or request label Mar 4, 2024
@andrea-pasquale andrea-pasquale self-assigned this Mar 4, 2024
Copy link

codecov bot commented Mar 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.86%. Comparing base (acef62e) to head (5d2e606).
Report is 2 commits behind head on fix_plot_chi2_protocols.

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                     @@
##           fix_plot_chi2_protocols     #745      +/-   ##
===========================================================
+ Coverage                    96.70%   96.86%   +0.15%     
===========================================================
  Files                          108      108              
  Lines                         7497     7490       -7     
===========================================================
+ Hits                          7250     7255       +5     
+ Misses                         247      235      -12     
Flag Coverage Δ
unittests 96.86% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/qibocal/cli/report.py 100.00% <100.00%> (+16.66%) ⬆️

Comment on lines +26 to +29
if target not in node.results:
# plot acquisition data and message for unsuccessful fit
figures = node.task.operation.report(data=node.data, fit=None, target=target)[0]
return figures, "An error occurred when performing the fit."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unaware we already stored this information somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also had to think about it. We can separate the two cases since only with qq acquire Results is None while in the other case the instance of the class is created and all the dict attributes are filled only if the fit is successful.

@andrea-pasquale andrea-pasquale added this to the Qibocal 0.0.8 milestone Mar 5, 2024
Base automatically changed from fix_plot_chi2_protocols to main March 8, 2024 20:50
@andrea-pasquale andrea-pasquale linked an issue Mar 8, 2024 that may be closed by this pull request
@andrea-pasquale andrea-pasquale added this pull request to the merge queue Mar 12, 2024
Merged via the queue into main with commit 1026f4f Mar 12, 2024
21 checks passed
@andrea-pasquale andrea-pasquale deleted the fit_error_message branch March 12, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add report failed information in the report.
3 participants