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

Adding logs #442

Merged
merged 8 commits into from
Jul 26, 2023
Merged

Adding logs #442

merged 8 commits into from
Jul 26, 2023

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Jul 12, 2023

This PR adds some logs in order to show which protocol is being executed.
The timing of both the acquisition and the fitting are shown for each protocol.
Here is an example

image

Checklist:

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

@andrea-pasquale andrea-pasquale added the enhancement New feature or request label Jul 12, 2023
@andrea-pasquale andrea-pasquale self-assigned this Jul 12, 2023
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #442 (75fc452) into main (a5cca61) will increase coverage by 0.01%.
Report is 119 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
+ Coverage   97.03%   97.05%   +0.01%     
==========================================
  Files          48       48              
  Lines        3107     3126      +19     
==========================================
+ Hits         3015     3034      +19     
  Misses         92       92              
Flag Coverage Δ
unittests 97.05% <100.00%> (+0.01%) ⬆️

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

Files Changed Coverage Δ
src/qibocal/auto/execute.py 98.73% <100.00%> (+0.03%) ⬆️
src/qibocal/auto/operation.py 96.09% <100.00%> (+0.59%) ⬆️
src/qibocal/auto/task.py 96.20% <100.00%> (ø)
src/qibocal/config.py 96.29% <100.00%> (ø)

@Jacfomg
Copy link
Contributor

Jacfomg commented Jul 12, 2023

Usually the fit takes almost nothing, could we add a check to only log that time when is above a threshold (1s maybe) ?

@rodolfocarobene
Copy link
Contributor

I kinda liked the "timing.json" file that we are producing for the benchmarks... Now that the the process is nicely written do you think we could have it directly in qibocal or is it useless?

@andrea-pasquale
Copy link
Contributor Author

Usually the fit takes almost nothing, could we add a check to only log that time when is above a threshold (1s maybe) ?

Yep, I can put a threshold. Maybe 1 second is too much (?)

@andrea-pasquale
Copy link
Contributor Author

I kinda liked the "timing.json" file that we are producing for the benchmarks... Now that the the process is nicely written do you think we could have it directly in qibocal or is it useless?

I need to see if there is a way to retrieve the information from the decorator. I feel like we don't need another json.
Perhaps we could store it somewhere else.

@andrea-pasquale
Copy link
Contributor Author

Just to a give an update now the timings are stored in the meta.json file (formerly it was a yaml but since it should not be modified by the user I decided to converted it to json.)
I've also added a threshold as requested .
Thanks @rodolfocarobene for the review already.

@andrea-pasquale andrea-pasquale requested review from Jacfomg and removed request for Jacfomg July 26, 2023 06:50
Copy link
Contributor

@Jacfomg Jacfomg left a comment

Choose a reason for hiding this comment

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

Great thanks!

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Jul 26, 2023
Merged via the queue into main with commit 7ea80cd Jul 26, 2023
20 checks passed
@andrea-pasquale andrea-pasquale deleted the add_logs branch July 26, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants