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

Reducing import overhead #579

Merged
merged 2 commits into from
Oct 25, 2023
Merged

Reducing import overhead #579

merged 2 commits into from
Oct 25, 2023

Conversation

andrea-pasquale
Copy link
Contributor

After talking with @stavros11 we realized that now importing qibocal takes longer than usual.
After profiling the import we discover that importing skops introduces an overhead of around 2 seconds.
Here is the profiling with skops:
image
This PR reduces the import time by moving skops as a relative import.
Here is the updated profiling:
image

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 Oct 24, 2023
@andrea-pasquale andrea-pasquale added this to the Qibocal 0.0.5 milestone Oct 24, 2023
@andrea-pasquale andrea-pasquale self-assigned this Oct 24, 2023
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #579 (b76923d) into main (96cd4d8) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
- Coverage   96.02%   95.99%   -0.04%     
==========================================
  Files          90       90              
  Lines        6062     6063       +1     
==========================================
- Hits         5821     5820       -1     
- Misses        241      243       +2     
Flag Coverage Δ
unittests 95.99% <100.00%> (-0.04%) ⬇️

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

Files Coverage Δ
src/qibocal/fitting/classifier/qubit_fit.py 100.00% <100.00%> (+3.63%) ⬆️
src/qibocal/fitting/classifier/run.py 96.69% <ø> (ø)

... and 1 file with indirect coverage changes

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.

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 @andrea-pasquale. I see a decrease of 1+ sec on total execution time when I run with dummy on my machine, maybe more in the cluster.

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Oct 25, 2023
Merged via the queue into main with commit 7893a8c Oct 25, 2023
21 checks passed
@andrea-pasquale andrea-pasquale deleted the fix_overhead branch October 25, 2023 11:07
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