-
Notifications
You must be signed in to change notification settings - Fork 328
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
Improve QSVR unit tests #434
Comments
Can we use these datasets in the TestQSVR unit test to fix this issue? |
These are too large for unit tests. Most likely an artificial one, say, size of <10 samples should be good. Or maybe even smaller, right now there 4 training samples and 2 test. |
Hi @adekusar-drl I am working on this issue at the moment |
@AbdullahKazi500 do you have ideas what exactly to be fixed in this issue? |
Can you assign me the issue |
Sure, I can. But the question I posted above is still valid. |
first Choosing an appropriate regression dataset that is small enough to be used for unit tests, but still representative of the regression problem. then Create a new unit test module or modify the existing TestQSVR module to include the new test cases. This module should import the necessary modules for dataset loading and splitting, and the QSVR implementation to be tested. Run the unit tests and make sure that they all pass. |
Thanks for getting back. A few comments:
So, the work should be pretty straight forward. Let me know if you have questions. |
I will keep you updated |
Hey @adekusar-drl Regarding the size of the dataset, you mentioned that a small dataset with a couple of data points should be sufficient for the test. As for the metrics, you suggested that one metric is enough, and currently, the test compares the evaluated score with the true value. In terms of hyperparameterization, are you sure if it is necessary for this specific test case. It may depend on the nature of the test and the algorithm being tested. However, if hyperparameterization is not required, then it is not necessary to include it in the test. ? Can you clarify this |
In general, there's no need for hyperparameterization. Just to be sure we are on the same page, what hyper parameters are you considering? |
I Guess it wont be necessary right ? For the Variational Quantum Classifier (VQC), hyperparameters include the number of layers in the quantum circuit, the depth of each layer, the type of parameterized gates used in each layer, and the number of training iterations. For the Quantum Kernel Estimator (QKE), hyperparameters include the type of quantum kernel used, the regularization strength, and the number of training iterations. |
No, all of them should fixed to certain values and that's it. Tests must check that algorithms work and results are meaningful, but no more. |
@AbdullahKazi500 is there any progress here? |
Hi @adekusar-drl I am Comparing the performance metrics with the expected values and check if they are within acceptable thresholds. and then I Run the unit tests and make sure that they all pass. |
Hi @AbdullahKazi500 has there been any recent progress? If so, please organize and submit the commits in a PR. |
yes I will be submitting a PR @edoaltamura |
I haven't heard recent updates on this test update and wanted to touch base and share a few timeline considerations FYI. We aim to include this update in the v.0.8 initial release, which is expected around the end of October 2024. This means that we would like to support long-standing community contributions - like this one - pull-requested by Friday 25 October, and leave enough time to coordinate the merging and release preparation. @AbdullahKazi500 Are you still available to work on this test with the suggestions in #434 (comment) by then or rather step out on this occasion? |
What is the expected enhancement?
Currently unit test in TestQSVR are based on the classification dataset, e.g., labels are
[0, 0, 1, 1]
. Such a dataset is not very well suited to test regressors. This issue intends to fix this problem and implement unit tests on a more suitable dataset.The text was updated successfully, but these errors were encountered: