-
Notifications
You must be signed in to change notification settings - Fork 160
Conversation
|
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.
Thank you very much for your contribution. This is looking very good.
On top of the requested changes, could you also add the following to the example notebook (it is in qiskit-ignis/examples/DiscriminatorTutorial.ipynb) to highlight your work?
from sklearn.svm import SVC
svcs = {
0: SVC(C=1., kernel="rbf", gamma="scale"),
1: SVC(C=1., kernel="rbf", gamma="scale")
}
svc_discriminators = {}
for q in qubits:
svc_discriminators[q] = SklearnIQDiscriminator(svcs[q], result, [q], ['0', '1'])
fig, ax = plt.subplots(1, 2, figsize=(14,5))
svc_discriminators[0].plot(ax[0], flag_misclassified=True, show_boundary=True)
svc_discriminators[1].plot(ax[1], flag_misclassified=False, show_boundary=True);
I suggest adding this at the end of the section entitled Single-qubit discriminator
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.
Made a suggestion to make is more transparent to the user what we require from the classifier
instance.
@eggerdj Thanks for the review! I've added the section to the tutorial notebook, but I don't think I have pulse-level access to any multi-qubit machine, so I'm unsure how to run the notebook cleanly and generate new output. Any suggestions? |
I think all comments have been addressed and that this is ready for further review. |
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.
@hodgestar This looks good. A few minor details: lint checks are failing.
Regarding the IQ data: there is a pickled data set test_result.pickle
in the example folder which contains data from one of the IBM Q backends. You can directly load this without having to interact with the backend by running the cell with
if use_prerun_data:
#Use pickle to load existing data
import pickle
from qiskit.result import Result
with open('test_result.pickle', 'rb') as handle:
res = pickle.load(handle)
result = Result.from_dict(res)
else:
result = job.result(timeout=3600)
This will allow you to test all sorts of classifiers.
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.
Code looks good but style checks are failing (see https://travis-ci.com/Qiskit/qiskit-ignis/jobs/267623281). Please see suggestions below.
Linting issues fixed (passing locally, waiting for Travis to finish). I updated the discriminator tutorial output. Note: Updating the notebook isn't quite trivial because even with |
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.
Looks good to me. Thanks a lot for this, really cool stuff.
@hodgestar You actually don't need to run the schedules or define them. If you just want to play with the pickled data you need to run only the imports, this block:
and the data loading part. Then you can create discriminators. |
@dcmckayibm This looks good. I am happy to merge this. |
@eggerdj Re notebook: Yep, that's what I did, but it's nice to be able to just restart & run all so that one has confidence the notebook runs correctly end to end. Anyway, it's good for now I think. Thanks for reviewing! |
@eggerdj @dcmckayibm Re-pinging this -- and happy new year! |
Summary
This PR adds a new measurement IQ discriminator that accepts any sklearn classifier as an input and uses that classifier to categorize measurements.
Details and comments
This is my first qiskit PR, so feedback welcome!
I couldn't find where the measurement docs should live. If someone lets me know where to start some, I can add them as part of this PR.