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

Fix update not working using qq fit #536

Merged
merged 4 commits into from
Oct 4, 2023
Merged

Fix update not working using qq fit #536

merged 4 commits into from
Oct 4, 2023

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Sep 25, 2023

While performing some tests I noticed that when using qq fit the update is not performed correctly.
This PR fixes this issue and it also simplifies the structure of Task.

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 bug Something isn't working label Sep 25, 2023
@andrea-pasquale andrea-pasquale self-assigned this Sep 25, 2023
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #536 (d8bec37) into main (aa89d58) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #536      +/-   ##
==========================================
+ Coverage   96.71%   96.72%   +0.01%     
==========================================
  Files          74       74              
  Lines        5380     5376       -4     
==========================================
- Hits         5203     5200       -3     
+ Misses        177      176       -1     
Flag Coverage Δ
unittests 96.72% <100.00%> (+0.01%) ⬆️

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

Files Coverage Δ
src/qibocal/auto/task.py 100.00% <100.00%> (+0.70%) ⬆️

@andrea-pasquale andrea-pasquale changed the title Fix update on multiple protocols Fix update not working using qq fit Sep 28, 2023
Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix. I am not very familiar with the auto part of the code, so it would be great if someone more familiar with it also reviews, but I do not see anything annoying in terms of code.

I also tested the update with one and two routines in the runcard and it worked as expected for the dummy platform. On main it does not work in either case. It may be useful to add a test that checks whether update works.

@andrea-pasquale andrea-pasquale requested review from Edoardo-Pedicillo and removed request for Edoardo-Pedicillo October 3, 2023 13:02
Copy link
Contributor

@Edoardo-Pedicillo Edoardo-Pedicillo left a comment

Choose a reason for hiding this comment

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

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Oct 4, 2023
Merged via the queue into main with commit 1eadcc9 Oct 4, 2023
20 checks passed
@andrea-pasquale andrea-pasquale deleted the fix_fit_builer branch October 4, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants