-
Notifications
You must be signed in to change notification settings - Fork 131
SVD data processing #48
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
Conversation
* Added tests. * Added train method to DataAction.
|
This is needed for #31 |
* Added SVD training test. * Added required dimension to SVD.
* Added error propagation mechanisme to the data processor. * Added optional outcome to Probability. * Added AverageIDData Node. * Added averaging and SVD test.
* Black and lint.
|
Can we have an option to "self-train" this from the data source? This is often good enough if your data set has a reasonable mixture of 0s and 1s. |
|
In #22 we decided to have the data processor apply to an individual datum, i.e. |
nkanazawa1989
left a comment
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.
Overall this looks good.
Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
chriseclectic
left a comment
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'm not sure about the changes to the base class here which seem to be very tied to this specific SVD function. Training doesn't seem like it is a general enough property for the base class.
Also do we need to allow runtime options for these objects, or can we take it in a direction of specifically setting options for node and data processors at initialization or using set_options? Passing generic kwargs through the entire processing chain might lead to some issues later on similar to the current issues in experiment classes with the various layers of options.
|
Here is some rational for the current implementation. Consider the case of spectroscopy. When running spectroscopy you have no pi-pulse and cannot run any calibrations. Therefore, you need to train your SVD (or whatever projection method you use) on the data you measure in the spectroscopy experiment. Therefore the data processor needs to self train on the data it is given. This would look like this: processor = DataProcessor("memory", [SVD()])
processor.train(experiment_data.data())
y_sigmas = np.array([processor(datum) for datum in experiment_data.data()]) |
…iments into svd-processing-node
nkanazawa1989
left a comment
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.
LGTM
chriseclectic
left a comment
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 better with the subclasses. One question is does is_trained need to be an abstract method? Or can the object just have an attribute like self._trained = False that this property returns, and the subclasses should set this to True as part of the train abstract method implementation?
|
One advantage with the abstract property is that it forces the person implementing the node to carefully think about what it means for the node to be trained. |
* * First draft of SVD node. * Added tests. * Added train method to DataAction. * * Added node and data processor training functionality. * * Fixed bug in _call_internal. * Added SVD training test. * Added required dimension to SVD. * * Added run-time options to data processor * Added error propagation mechanisme to the data processor. * Added optional outcome to Probability. * Added AverageIDData Node. * Added averaging and SVD test. * * Added error propagation for SVD node and tests. * Black and lint. * * Added call to super().setUp() * * Removed redundent setUp * * Made the averaging node independent of IQData. * * Docstring. * * Black * * Lint * * Black. * * Fix docstring. * Update qiskit_experiments/data_processing/data_processor.py Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com> * * Made means a function to improve readability. * * Removed RealAvg and ImagAvg. * * Added TrainableDataAction as a subclass of DataAction. * * Removed _required_dimension. * * Removed **options from data processing. * * Used the property * * Removed raises in SVD. * * Made scale default to 1.0 Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
Summary
This PR implements a
Details and comments
The SVD allows the user to project the IQ data on the main axis of the data. This also means that this axis must be determined before using the SVD. Furthermore, this training is not done on a single datum but either on all the data or a subset of the data (the data processing calibration data).
DataProcessors can now be trained one node at a time using thetrainmethod. Each node has ais_trainedproperty which by default isTrue. Nodes that require a training process must overwrite this property.The error propagation and run-time options are implemented by changing the signature of
DataActioncalls fromto
and making the required changes in
DataProcessor.TODO: