Params from parent java estimators aren't copied to python mmlspark models

[kschelonka]

The java params for mmlspark estimators like LightGBMClassifier, etc. aren't copied over to the python instances.

This is related to this Jira ticket: PySpark ML Models should contain Param values

A temporary fix was added so that the params can be accessed using getOrDefault method. This does make it possible to pull in the params from mmlspark models, like LightGBMClassifier:

Spark developers are planning on incrementally updating the pyspark API to use the appropriate getter and setter methods, and having the pyspark models define the params within themselves (see SPARK-21812). For example, CountVectorizer was updated in this fashion.

Since it's very useful to be able to access model parameters, I propose updating mmlspark models in a similar fashion. Happy to contribute to this effort.

[imatiach-msft]

@kschelonka thanks for bringing up this issue!
"having the pyspark models define the params within themselves"
this sounds reasonable, although I think we should only allow the parameters to be settable on the model if they can actually change the output of the model (I recall seeing cases in Spark ML where this was not the case which seemed strange to me).
"I propose updating mmlspark models in a similar fashion. Happy to contribute to this effort."
that would be great, if you have the cycles to do so. Note that all of our python wrappers are autogenerated - so this should hopefully require less coding and work on all python models at once.
Also, I should mention that @mhamilton723 is doing a large refactor of the codebase right now. Currently, we have a special ./runme script that sets up mmlspark on linux, but it relies on bash and may do too much on behalf of the developer. @mhamilton723 is refactoring the codebase to rely only on sbt which should make it possible to develop on windows and mac os. Setting up the code for development is a bit difficult right now. He might suggest that you use the refactor branch, I'm not sure how far along he is in the refactor.

[kschelonka]

Thanks for the response!
"we should only allow the parameters to be settable on the model if they can actually change the output of the model "
That makes sense to me as well, but I would suggest that the settable params should include anything that has no effect on model fit. So for example featuresCol should be able to be settable, even though it doesn't change the model's output.

The build refactor sounds like it would make development easier, so I'll wait for updates on that. Thanks!

[mhamilton723]

@kschelonka, @imatiach-msft I think this would be mitigated by making the LightGBM models proper sparkML estimators with getter and setters instead of being constuctor defined. This would also improve code re-use. Ilya, could you consider this when doing yout LGBM cleanup PR?

[imatiach-msft]

FYI this is now fixed on LightGBM (because of ComplexParamsWritable) but not on some of the other estimators like TrainClassifier.