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

Changing to_dict in property #299

Merged
merged 5 commits into from
Apr 14, 2023
Merged

Changing to_dict in property #299

merged 5 commits into from
Apr 14, 2023

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Apr 10, 2023

This PR needs to be merged as soon as we merge qiboteam/qibolab#346, which will change the to_dict method into the property raw. This should resolve all issues related to the attribute average of the to_dict method.

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Merging #299 (0a8bd52) into main (9a27c90) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #299   +/-   ##
=======================================
  Coverage   37.84%   37.84%           
=======================================
  Files          42       42           
  Lines        2801     2801           
=======================================
  Hits         1060     1060           
  Misses       1741     1741           
Flag Coverage Δ
unittests 37.84% <ø> (ø)

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

@andrea-pasquale
Copy link
Contributor Author

I manage to test on qblox the majority of the routines (I am missing the ones involving sampling, two qubit gates and rb).
@stavros11 @rodolfocarobene @Jacfomg when you have time could you please test this branch with qiboteam/qibolab#346 with qm, rfsoc and zurichs? Let me know if this fixes the issues related to the method to_dict.

@rodolfocarobene
Copy link
Contributor

rodolfocarobene commented Apr 11, 2023

I just tried it and it fixes the issue.
However I had to do a couple of things in the qibolab PR (merge main and initialize all AveragedResults with from_components in the rfsoc driver).

If you want I can push those changes, the merge was without conflicts

@andrea-pasquale
Copy link
Contributor Author

I just tried it and it fixes the issue. However I had to do a couple of things in the qibolab PR (merge main and initialize all AveragedResults with from_components in the rfsoc driver).

If you want I can push those changes, the merge was without conflicts

Thanks for the feedback @rodolfocarobene.
Feel free to push those changes in qiboteam/qibolab#346.

@andrea-pasquale andrea-pasquale self-assigned this Apr 11, 2023
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.

@Edoardo-Pedicillo
Copy link
Contributor

I confirm that it works with dummy.

@andrea-pasquale
Copy link
Contributor Author

@stavros11 I would like to merge this but I'm not able to test it on QM since I don't know exactly what branch to use. I know that the runcard is in qiboteam/qibolab#349 and that you started from main with qiboteam/qibolab#361, however this branch works with qiboteam/qibolab#346 which is also pointing to main.
Given that qiboteam/qibolab#346 does not introduce major changes if you agree we could merge it to main and you can later test it on qiboteam/qibolab#349 or qiboteam/qibolab#361.

@stavros11
Copy link
Member

@andrea-pasquale I wanted to test it yesterday but unfortunately the QM went out and as far as I know we have not been able to connect to it yet. I am not sure when it will be back but at this stage we could merge this and qiboteam/qibolab#346 and if something breaks for QM I’ll fix it in one of my other open PRs in qibolab.

@andrea-pasquale
Copy link
Contributor Author

@andrea-pasquale I wanted to test it yesterday but unfortunately the QM went out and as far as I know we have not been able to connect to it yet. I am not sure when it will be back but at this stage we could merge this and qiboteam/qibolab#346 and if something breaks for QM I’ll fix it in one of my other open PRs in qibolab.

Thanks. I'll proceed with the merge as soon as we merge qiboteam/qibolab#346.

@andrea-pasquale andrea-pasquale merged commit 598e862 into main Apr 14, 2023
@andrea-pasquale andrea-pasquale deleted the from_to_dict_to_serial branch April 14, 2023 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants