zhipeng93 commented on a change in pull request #41:
URL: https://github.com/apache/flink-ml/pull/41#discussion_r772769059



##########
File path: 
flink-ml-iteration/src/main/java/org/apache/flink/iteration/Iterations.java
##########
@@ -261,10 +256,6 @@ private static DataStreamList createIteration(
         }
 
         List<DataStream<?>> tailsAndCriteriaTails = new 
ArrayList<>(tails.getDataStreams());
-        checkState(

Review comment:
       Do we suppose to support termination criteria in unbouned iterations? If 
we are not doing this, we should probably keep this check.

##########
File path: 
flink-ml-lib/src/main/java/org/apache/flink/ml/classification/logisticregression/LogisticRegression.java
##########
@@ -362,7 +362,6 @@ private void updateModel() {
         @Override
         public void onEpochWatermarkIncremented(
                 int epochWatermark, Context context, Collector<double[]> 
collector) {
-            // TODO: let this method throws exception.

Review comment:
       Could you also update this method and do not throw runtime exception in 
this method?

##########
File path: 
flink-ml-iteration/src/main/java/org/apache/flink/iteration/Iterations.java
##########
@@ -117,19 +117,16 @@
      */
     public static DataStreamList iterateUnboundedStreams(
             DataStreamList initVariableStreams, DataStreamList dataStreams, 
IterationBody body) {
-        return createIteration(
+        return iterateStreams(

Review comment:
       After this renaming, having `iterateUnboundedStreams` and 
`iterateStreams`, without `iterateBoundedStreamsUntilTermination` seems a bit 
confusing to me. These APIs are not symmetric for bounded and unbounded cases. 
A side effect of doing so is that we cannot restrict the following use case: 
supporting termination criteria on iterating unbounded streams.
   
   
   Could you explain why do you propose to make the change here?




-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to