-
Notifications
You must be signed in to change notification settings - Fork 4
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
qq compare command to compare two qibocal reports #714
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
+ Coverage 97.15% 97.21% +0.05%
==========================================
Files 100 101 +1
Lines 7415 7533 +118
==========================================
+ Hits 7204 7323 +119
+ Misses 211 210 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Missing |
Hi @GabrielePalazzo, what is the status of this PR ? |
Comparison of single sweeper experiments (like resonator spectroscopy) works. |
I agree. If you want, you can just put the plots of the two reports near each other. |
I was going to suggest the same. Another possible idea would be to come up with some sort of ratio plot to see the differences between the two outcomes. I don't know how feasible it is. |
The new code is not covered by tests. I'll try to write some. |
@GabrielePalazzo, could you resolve the conflicts? Thanks |
For conflicts let me know if you need help. I recently changed some stuff related to the report generation. |
@@ -36,6 +36,7 @@ seaborn = { version = "^0.12.2", optional = true } | |||
pydot = { version = "^1.4.2", optional = true } | |||
skl2onnx = { version = "^1.14.0", optional = true } | |||
pyyaml = "^6.0" | |||
lxml = "^5.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not with standard library? (maybe I just forgot...)
It seems reliable, and it's only for reporting, so it should be alright (at some point, we could decouple the UI from the library - CLI included - and this dependency would just be in the front-end), but one (compiled) dependency less is always better :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, for some reason I found it easier to use lxml
for manipulating the html code, but I can try to switch to a standard package.
html_block = lxml.html.fromstring(plots[0][0].to_html()) | ||
html_block[1].remove(html_block[1][0]) | ||
for i, plot in enumerate(plots): | ||
single_report_html_block = lxml.html.fromstring(plot[0].to_html()) | ||
html_block[1].insert(i, single_report_html_block[1][0]) | ||
fig = lxml.etree.tostring(html_block).decode("ascii") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to my previous comment.
The explicit HTML manipulation seems to be localized here.
Isn't it possible to avoid the HTML manipulation at all?
Moreover, it seems that the function is:
- extracting the original HTML from the first plot, then into an object
- removing the plot itself (I guess)
- reinserting all the plots in the HTML as consecutive nodes
- converting back to an HTML string
Can't you just manipulate the Plotly objects, and stack them vertically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I remember, vertically stacking two plots from HTML reports was giving weird layouts (e.g.: plots overlapping one on top of the other).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, doing something like this
https://plotly.com/python/subplots/#stacked-subplots
Since, initially, you should have the Plotly objects (those on which you're calling the .to_html()
method)
Example of a combined report containing a resonator spectroscopy and a resonator flux dependence:
http://login.qrccluster.com:9000/T5rbRzsmTRmKLtOSaa03DQ==
Only experiments with the same id are compared. All other experiments are discarded.
Tables contain results from both experiments side-by-side.
Experiments with scatterplots (e.g.: resonator spectroscopy) are shown in the same figure (with overlapping points and lines).
Plots of experiments with heatmaps (e.g.: resonator punchout) are vertically stacked.
Checklist:
master
main
main