Copied changes to readoutAngle from qiskit-monitoring to qiskit-exper…#586
Conversation
…iments co-authored-by mattias.fitzpatrick@ibm.com
yaelbh
left a comment
yaelbh
left a comment
There was a problem hiding this comment.
I see that it's still WIP (WIP in the title, lint, tests). Anyway when you're closer to get done there are a couple of things that I suggest to do:
- Save an experiment to the result db, check that it looks ok on the web (that the analysis results and figures are fine), load and verify that the loaded experiment is identical to the saved one.
- Check if other custom analysis should be migrated to qiskit-experiments as well.
| from qiskit_experiments.framework import BaseAnalysis, AnalysisResultData | ||
|
|
||
| from qiskit_experiments.framework import BaseAnalysis, AnalysisResultData, Options | ||
| from qiskit_experiments.framework.matplotlib import get_non_gui_ax |
There was a problem hiding this comment.
I'm not familiar with this one, so someone who's familiar should check it. If other experiments work in other ways then it's suspicious.
There was a problem hiding this comment.
- I ran the experiment on a device and verified the results.
- How do you load an experiment from the web?
There was a problem hiding this comment.
Regarding get_non_gui_ax, I found this was used by curve_analysis. Perhaps @nkanazawa1989 can answer whether this is correct usage here.
There was a problem hiding this comment.
Regarding your comment @yaelbh about other moving other customAnalysis functions, I will do that in a separate PR.
There was a problem hiding this comment.
@nkanazawa1989 , can you please check if the usage of get_non_gui_ax is correct here?
| ] | ||
| name="readout_angle_qiskit_monitoring", | ||
| value=angle), | ||
| AnalysisResultData( |
There was a problem hiding this comment.
I'm not sure if what we want is multiple analysis results or one result with an extra parameter that stores all the extra information. Maybe consult with Mattias and Will.
There was a problem hiding this comment.
Hmm, I haven't heard much discussion in general about using separate analysis results vs. extra fields of a single result in qiskit-experiments. What makes sense to me is to use separate AnalysisResultData entries for quantities that are of interest independently and extra for quantities that are only of interest in the context of another result. Here, I think readout_angle is the only quantity of interest outside of the analysis (the others are helpful for debugging or qualifying the result value), so I would make the others extras. These quantities could be of interest for calculating something else like readout SNR, but that would probably be done in a separate analysis class.
merav-aharoni
changed the title
|
@yaelbh - can you approve this PR? |
yaelbh
left a comment
yaelbh
left a comment
There was a problem hiding this comment.
Did you save an experiment to the result db, check that it looks ok on the web (that the analysis results and figures are fine), load and verify that the loaded experiment is identical to the saved one?
| from qiskit_experiments.framework import BaseAnalysis, AnalysisResultData | ||
|
|
||
| from qiskit_experiments.framework import BaseAnalysis, AnalysisResultData, Options | ||
| from qiskit_experiments.framework.matplotlib import get_non_gui_ax |
|
@yaelbh - 'Yes' to your first question, 'no' to your second question (I already asked twice). |
qiskit-community#586) * Copied changes to readoutAngle from qiskit-monitoring to qiskit-experiments co-authored-by mattias.fitzpatrick@ibm.com * lint and black * Removed qiskit_monitoring from the names * Moved all results except for readout_angle, to be extra_results
qiskit-community#586) * Copied changes to readoutAngle from qiskit-monitoring to qiskit-experiments co-authored-by mattias.fitzpatrick@ibm.com * lint and black * Removed qiskit_monitoring from the names * Moved all results except for readout_angle, to be extra_results
3 participants
…iments
co-authored-by mattias.fitzpatrick@ibm.com
Summary
Details and comments