Improve handling of missing optimizer in NN classifier/regressor

[qiskit-community]

Improve handling of missing optimizer in NN classifier/regressor

[woodsp-ibm]

There is information around contributing here https://github.com/Qiskit/qiskit-machine-learning/blob/main/CONTRIBUTING.md#contributing which also links to the main Qiskit contributing docs that have a lot of common information.

The easiest way to get the tools you need for development installed is to do a pip install -r requirements-dev.txt - you will see the file in the root of the repo. It has the list of all the tools needed - because they are only needed if you are doing dev they are not part of the standard requirements/install.

[darshkaushik]

This sounds like a super interesting introduction to this repo, any chance I could take this issue over @darshkaushik

@mattwright99, I have already invested several hours and would like to continue with it, hope that's fine

@darshkaushik how is your progress working on this issue?

Have gone through VQE and hope to commit by today, will let you know if I get stuck somewhere.

[adekusar-drl]

@darshkaushik thanks, I'll keep this issue on you.

[adekusar-drl]

Yeah, please install it via pip install pylintfileheader. You should also have qiskit-terra installed in your python environment.

[adekusar-drl]

This is likely due to different line endings in linux and windows: \n vs \r\n. I guess you did not edit copyright notices, so ignore them.

[woodsp-ibm]

In looking at the code there are other params too with defaults of None where the typehint does not define them as Optional. I can see None being a valid value for optimizer if internally we then create some default Optimizer for convenience. Otherwise it seems to me that its being defaulted to an invalid value.
If some are left Optional I think we need to check the typehints on some of the getters to mark them Optional too - of course if we default an Optimizer on construction then the setter would not need to be type hinted as Optional.

[darshkaushik]

@adekusar-drl I would like to contribute to fix this issue.
Are the changes to be done in classifier/regressor or the parent class TrainableModel ?
NeuralNetworkClassifier has an overriden __init__ method whereas NeuralNetworkRegressor does not.

Also if we choose to have a default value for the parameters, on what basis should they be chosen is a little ambiguous.

[adekusar-drl]

Thanks for taking care of this! I think this issue should be fixed in TrainableModel as it is a super class for both regressor/classifier and there's optimizer property in it. A default value and behavior may be similar to what we have in VQE, e.g. let's start from SLSQP(although it may be not the best choice) as a value, if it does work well by default in the tests, then you may switch to BFGS/Cobyla as they are heavily used in the tests. Also, you may explore VQE to understand how it should look like.

[mattwright99]

This sounds like a super interesting introduction to this repo, any chance I could take this issue over @darshkaushik

[adekusar-drl]

@darshkaushik how is your progress working on this issue?

[darshkaushik]

@adekusar-drl please review the changes.

Also, it being my first issue, I need some help on how to run tests and style checks on a windows system.

[adekusar-drl]

@darshkaushik Could you please create a PR for your changes? If it is not yet fully ready, you can create a draft PR and/or mark with a prefix "[WIP]" in the title, work in progress.

Usually I run tests from PyCharm directly, it's pretty easy. To run various checks you can install make on windows and run the makefile. Or you can run manually command from the makefile, e.g. pylint -rn qiskit_machine_learning test tools for linting, black --check qiskit_machine_learning test tools to check style and so on.

[darshkaushik]

@adekusar-drl I tried using pylint command directly from the qiskit-machine-learning directory in my local system, but I always get the error ModuleNotFoundError: No module named 'pylintfileheader'. This error comes only in this particular directory, in every other directory pylint works fine and I can't find a fix for it.

Also, I have cloned just the qiskit-machine-learning repo, without qiskit-terra, but I don't think that it would be the reason for it.

[darshkaushik]

when linting got invalid-file-header for all the files, why is it so?

[mtreinish]

The pylint header extension is quite error prone in my experience and doesn't give you any real debugging information. This is why other projects use the verify_headers.py script I wrote to do this. It's faster and provides real debug information on why a header is invalid.

[darshkaushik]

other projects use the verify_headers.py script I wrote to do this.

@mtreinish where can I find it?

@adekusar-drl I did the unit tests they work fine for all the files in the algorithms directory, but others do not, I hope that's okay as it is unrelated to this fix.

[mtreinish]

@darshkaushik it's apparently not included in machine-learning repo we'll have to port it from another qiskit repository like: https://github.com/Qiskit/qiskit-terra/blob/main/tools/verify_headers.py it should be a separate PR moving away from the pylint plugin to that script.

[darshkaushik]

I hope the the changes look fine. Let me know if you have any suggestions.