madlib-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [madlib] kaknikhil commented on a change in pull request #522: DL: Remove keras dependency
Date Sat, 17 Oct 2020 00:51:17 GMT

kaknikhil commented on a change in pull request #522:
URL: https://github.com/apache/madlib/pull/522#discussion_r505089686



##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_wrapper.py_in
##########
@@ -248,7 +249,7 @@ def parse_optimizer(compile_dict):
     opt_split = compile_dict['optimizer'].split('(')
     opt_name = opt_split[0]
     optimizers = get_optimizers()
-    _assert(opt_name in optimizers,
+    _assert(opt_name.lower() in [o.lower() for o in optimizers.keys()],

Review comment:
       what if opt_name is None ?

##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras.py_in
##########
@@ -44,7 +39,15 @@ from utilities.utilities import madlib_version
 from utilities.utilities import unique_string
 from utilities.validate_args import get_expr_type
 from utilities.control import MinWarning
+
 import tensorflow as tf
+from tensorflow import keras

Review comment:
       We weren't explicitly importing keras before, is this for the case when user has a
`keras` dependency in their model json ?

##########
File path: src/ports/postgres/modules/deep_learning/test/madlib_keras_fit.sql_in
##########
@@ -397,36 +397,3 @@ SELECT assert(
     class_values = '{NULL,0,1,4,5}',
     'Keras model output Summary Validation failed. Actual:' || __to_char(summary))
 FROM (SELECT * FROM keras_saved_out_summary) summary;
-

Review comment:
       I think it will help to add a note to the commit message explaining why we removed
these tests 




----------------------------------------------------------------
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:
users@infra.apache.org



Mime
View raw message