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

Qibolab benchmarks #433

Closed
wants to merge 29 commits into from
Closed

Qibolab benchmarks #433

wants to merge 29 commits into from

Conversation

rodolfocarobene
Copy link
Contributor

Replaces #415 for the benchmarks
I merged main and some fix for dispersive shift. Now the execution produces a file called "timings.json" with the important info.

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #433 (49450e5) into main (0e22736) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 49450e5 differs from pull request most recent head c112b45. Consider uploading reports for the commit c112b45 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
+ Coverage   96.77%   96.78%   +0.01%     
==========================================
  Files          48       48              
  Lines        3129     3145      +16     
==========================================
+ Hits         3028     3044      +16     
  Misses        101      101              
Flag Coverage Δ
unittests 96.78% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
src/qibocal/auto/execute.py 98.91% <100.00%> (+0.22%) ⬆️

@Jacfomg
Copy link
Contributor

Jacfomg commented Jul 13, 2023

What do you think about those scans for the scaling ? I need to reduce the rabi amp points I think.

@rodolfocarobene
Copy link
Contributor Author

What do you think about those scans for the scaling ? I need to reduce the rabi amp points I think.

I think it's useful to always have the 1 (a single point in the sweep) that more or less is equal to the overhead for an execution. Then I would propose to have, for all the parameters, the same numbers of point in power of 10: 1, 10, 100, 1000 (eventually also the 10.000 point but could become too slow).
In this way the plots would look kinda nice in log scale

@rodolfocarobene
Copy link
Contributor Author

@stavros11 @Jacfomg I've pushed a runcard for the scalings in 1D. in every routine the points are 1,10,100,1000. For the single point sweeper sometimes there is the need of changing the fit functions so that they do not fail.

With a timings.json for every type of sweeper is very easy to achieve something like this:
image

If you can execute them it would be nice, if you have other proposals please feel free to propose them :-)

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.

Thanks, I will try soon. The runcard is scaling/1d_sweepers.yml? There is another runcard for scaling in iqm that has different point values, we need to make sure everyone is using the same.

# max_amp_factor: 1.0000
# step_amp_factor: 1
# pulse_length: 40
# relaxation_time: 5_000
Copy link
Member

Choose a reason for hiding this comment

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

The relaxation times here are different than the other benchmark. Since we are not using a qubit it is fine to do even Rabi with very short relaxation time, just need to make sure we report it properly in the paper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that was my point, but in any case we can change it.

I used scaling/1d_sweepers.yml that I derived from the one proposed by Juan

@andrea-pasquale
Copy link
Contributor

What is the plan for this PR @Jacfomg @stavros11?
Keep in mind that now the timings are stored into the meta.json file. See #442

@Jacfomg
Copy link
Contributor

Jacfomg commented Aug 21, 2023

What is the plan for this PR @Jacfomg @stavros11? Keep in mind that now the timings are stored into the meta.json file. See #442

This also saved times inside of the drivers I think, but in the end we decided not to use them. So, I would say that this PR is only important now because it has the runcards we used for the timings. We should save them somewhere.

@andrea-pasquale
Copy link
Contributor

This also saved times inside of the drivers I think, but in the end we decided not to use them. So, I would say that this PR is only important now because it has the runcards we used for the timings. We should save them somewhere.

I see, then I suggest to open a separate PR just to upload the runcards used in the qibolab paper.
Then if you agree we can probably close this.

@stavros11 stavros11 mentioned this pull request Aug 23, 2023
8 tasks
@andrea-pasquale andrea-pasquale deleted the benchmark branch August 23, 2023 09:49
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.

5 participants