[GitHub] artemmalykh commented on a change in pull request #5961: IGNITE-11222

[GitBox <gi...@apache.org>]

artemmalykh commented on a change in pull request #5961: IGNITE-11222
URL: https://github.com/apache/ignite/pull/5961#discussion_r251912276
 
 

 ##########
 File path: modules/ml/src/main/java/org/apache/ignite/ml/clustering/kmeans/KMeansTrainer.java
 ##########
 @@ -82,14 +76,18 @@
 
     /** {@inheritDoc} */
     @Override protected <K, V> KMeansModel updateModel(KMeansModel mdl, DatasetBuilder<K, V> datasetBuilder,
-        IgniteBiFunction<K, V, Vector> featureExtractor, IgniteBiFunction<K, V, Double> lbExtractor) {
+        IgniteBiFunction<K, V, SimpleLabeledVector<Double>> extractor) {
 
 Review comment:
   `SimpleLabeledVector` is a `LabeledVector` with fixed first type parameter. It was created just not to write both parameters in place of usage, from the other hand after your comment I've rechecked the code usages of `LabeledVector` and found out that `V` type parameter is never used in the sense that we nowhere care what exact `Vector` subclass it is, we treat it just a `Vector`, so maybe I will refactor `LabeledVector` and remove this type parameter and after that remove `SimpleLabeledVector` which will become a full duplicate?
   
   As for API. Separable extractors are expressed via inseparable but not vice versa in common case. That's why now the base method using inseparable extractors. From the other hand, you are right, all current trainers except boosting are fine with inseparable ones. Maybe we can add class like `SimpleDatasetTrainer` which has finalized methods with inseparable extractors expressed through abstract methods with separable extractors and subclass all our trainers from them? Idk, just didn't want to change class hierarchy...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services