reductionista commented on a change in pull request #525:
URL: https://github.com/apache/madlib/pull/525#discussion_r556075238



##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras.py_in
##########
@@ -520,21 +544,35 @@ def fit_transition(state, dependent_var, independent_var, 
dependent_var_shape,
         and only gets cleared in eval transition at the last row of the last 
iteration.
 
     """
-    if not independent_var or not dependent_var:
+    if not dependent_var_shape:

Review comment:
       As I understand it, we require the input table to be minibatched--so 
every input row must contain an array with the same dimensions.  And where the 
sizes of all but the first dimension of the arrays must match between all rows. 
 A NULL value for a class should be okay, and maybe even a NULL image somewhere 
in the array (although I'm not sure what that would mean), but I think if 
either the x or y array itself were NULL, or the shape is NULL, that's an 
invalid input since it wasn't minibatched properly.
   
   I'm not sure if we explicitly error out in the validation code during 
initialization in that case.  I figured if we were allowing that before is was 
probably unintentional.  Is there a reason why we would need to support that?




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


Reply via email to