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

Fast reset testing routine #399

Merged
merged 25 commits into from
Jul 24, 2023
Merged

Fast reset testing routine #399

merged 25 commits into from
Jul 24, 2023

Conversation

Jacfomg
Copy link
Contributor

@Jacfomg Jacfomg commented Jun 13, 2023

For now this can be used as a sanity check when playing with the fast reset capability of some devices. I'm still not sure what's the best thing for plotting or if it's needed for a good benchmark to measure both states.

Related to #435 as the results would be bad until we get a big QND-ness but the orutine works as expected.

Edit:
This PR is related to qiboteam/qibolab#506, meaning it only works with that one for Zurich. And qiboteam/qibolab_platforms_qrc#28
Checklist:

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

@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Merging #399 (7d3983f) into main (ec7e5cc) will increase coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head 7d3983f differs from pull request most recent head cfb8000. Consider uploading reports for the commit cfb8000 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
+ Coverage   97.15%   97.24%   +0.09%     
==========================================
  Files          49       50       +1     
  Lines        3233     3343     +110     
==========================================
+ Hits         3141     3251     +110     
  Misses         92       92              
Flag Coverage Δ
unittests 97.24% <100.00%> (+0.09%) ⬆️

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

Impacted Files Coverage Δ
src/qibocal/protocols/characterization/__init__.py 100.00% <100.00%> (ø)
...rotocols/characterization/fast_reset/fast_reset.py 100.00% <100.00%> (ø)

@Jacfomg Jacfomg marked this pull request as ready for review June 27, 2023 08:03
@Jacfomg
Copy link
Contributor Author

Jacfomg commented Jun 27, 2023

I have a problem, I wanted to get plotty to display a heatmap with text on it to plot a matrix with the state counts, but for some reason I can't make it work. Have you ever plotted something similar ?

@andrea-pasquale
Copy link
Contributor

I have a problem, I wanted to get plotty to display a heatmap with text on it to plot a matrix with the state counts, but for some reason I can't make it work. Have you ever plotted something similar ?

Perhaps @Edoardo-Pedicillo did something similar with single shot classification, I'm not so sure.

Copy link
Contributor

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

Thanks @Jacfomg.
You can find below some suggestions.

src/qibocal/protocols/characterization/fast_reset/utils.py Outdated Show resolved Hide resolved
Comment on lines +189 to +190
def _plot(data: FastResetData, fit: FastResetResults, qubit):
"""Plotting function for FastReset."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Also this function could be simplified I think. Lets try to fix first the issue related to the plotting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it will be easier for me to put it in loops as the acquisition once it's working

@Jacfomg Jacfomg marked this pull request as ready for review July 7, 2023 07:57
@Edoardo-Pedicillo
Copy link
Contributor

Perhaps @Edoardo-Pedicillo did something similar with single shot classification, I'm not so sure.

Not with plotly, I tried smething like

z=[[1, 20, 30],
    [20, 1, 60],
    [30, 60, 1]]

z_text = [[str(y) for y in x] for x in z]

fig.add_trace(

        go.Heatmap(
            z=z,
            text=z_text,
            texttemplate="%{text}",
            textfont={"size":20}
        ),
        row=1,
        col=1,
    )

it works locally (in a Jupyter Notebook), but not in the report, maybe you will find the error in the function that generate the reports.

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.

I suggest plotting the confusion matrix with the relative values, in this way you can use only one color legend.

Copy link
Contributor

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

Tested on hardware and it works.
Some other suggestions very similar to #435

Copy link
Contributor

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

Thanks @Jacfomg, just a very small suggestion.
Btw, I had to merge main both in qiboteam/qibolab#506 and qiboteam/qibolab_platforms_qrc#28 to make it work.

@andrea-pasquale andrea-pasquale merged commit 4bac506 into main Jul 24, 2023
@andrea-pasquale andrea-pasquale deleted the fast_reset branch July 24, 2023 13:35
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.

3 participants