xuyang1706 commented on issue #8632: [FLINK-12744][ml] add shared params in ml 
package
URL: https://github.com/apache/flink/pull/8632#issuecomment-501552635
 
 
   > @xuyang1706 Thanks for the patch. I made a quick pass and left some 
comments.
   > 
   > I have to admit that I do not have enough expertise to say whether the 
various params interfaces added in `o.a.f.ml.params` are necessary or not. It 
will be great if you can help the reviewer understand this better. One possible 
way is to enhance the Java doc for them. It might also be better to check in 
some of the specific `WithParams` subclasses together with the algorithm that 
actually uses them, instead of in this PR. Doing that would make it a lot 
easier to understand what those `WithParams` interfaces are for.
   > 
   > Please let me know what do you think.
   
   @becketqin Thanks for your comments. I removed the most of parameter info 
definitions, keep the common and typical ones which are helpful to understand 
current refactoring of ParamInfo and Params. The left parameter infos could be 
understandable by their name. The others will be added with according 
algorithms. Current codes might be easier for the reviewers. Thanks.

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


With regards,
Apache Git Services

Reply via email to