-
Notifications
You must be signed in to change notification settings - Fork 128
Discriminator Experiment Class #30
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
Discriminator Experiment Class #30
Conversation
It appears that you've implemented something for multiple experiments that are parallel independent experiments. If they are indeed independent, then the correct thing to do would be to implement circuit and analysis functions that see only one qubit, and to handle multiple qubits by the parallel experiment class. |
If the analysis function assumes that the circuits were executed with |
I see that |
Perhaps you can leverage the IQ test backend in the tutorial: https://github.com/Qiskit/qiskit-experiments/blob/960f6395db248711ec5d460e7371d275b041c2f6/qiskit_experiments/test/mock_iq_backend.py#L46 |
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.
This looks like it is going in the right direction. I think we can make the integration with the data processing nodes a bit leaner. For instance, it looks like one could combine the LDADiscriminator
and QDADiscriminator
nodes.
class DiscriminatorExperiment(BaseExperiment): | ||
"""Discriminator Experiment class""" |
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.
How would this work with say 0, 1, and 2? I.e. if we want to use a discriminator for a qutrit experiment. Will there be a new experiment or should we make the current one more flexible?
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.
The ESP discriminator is being implemented here: coruscating#4
Because it requires pulse, we felt it's significantly different from the standard discriminator and so it's a completely different class.
qiskit_experiments/measurement/discriminator/discriminator_experiment.py
Outdated
Show resolved
Hide resolved
…nts into move_backend
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.
Thanks for updating the code. Given how the code is written it looks like the Discriminate
node is not trainable so it should be Discriminate(DataAction)
. Do we want to allow untrained data processors to be used as nodes, i.e. Discriminate(TrainableDataAction)
? If so I would expect the processor to train itself on a subset of the data e.g. if an experiment contains circuits of the form:
# Calibration circuits
|0> - meas
|0> - X - meas
# Experiment circuits
Some cool circuits
then a discriminator could train itself.
"""Base class for discriminator processor. Takes IQ data and calibrated discriminator as input, | ||
outputs counts.""" |
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.
Can this be an untrained discriminator as the base class suggests it could be?
"""Base class for discriminator processor. Takes IQ data and calibrated discriminator as input, | ||
outputs counts.""" |
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.
Could this take an untrained discriminator as input as the base class would suggest?
"""Base class for discriminator processor. Takes IQ data and calibrated discriminator as input, | ||
outputs counts.""" | ||
|
||
def __init__(self, handle: ExperimentData, validate: bool = True): |
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.
What is the rational for giving the init a handle to the data? This deviates from the other nodes which are called on the data.
from .discriminator_analysis import DiscriminatorAnalysis | ||
|
||
|
||
class Discriminator(BaseExperiment): |
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.
Could we either change the name to indicate that this discriminates 0 and 1 or provide the functionality to extend this to higher excited states? I think changing the name would be sufficient at first.
"meas_level": self.run_options.meas_level, | ||
"meas_return": self.run_options.meas_return, |
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 believe we can remove these now. Is there a reason to keep them?
@coruscating Do you intend to complete this PR ? |
If not in the near future then we don't have to close it, we can keep it open and move it to the IceBox pipeline |
@yaelbh Yes, I'm working on the updates and almost done. |
Will open new PR for this. |
Summary
Basic discriminator class that implements LDA and QDA for multiple qubits. One calibration is done for |00..0> and |11..1> state, then each qubit is fitted separately. Discriminator data processing node, once trained, can be used to discriminate future data.