kaknikhil commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583335692
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras.sql_in
##########
@@ -996,22 +1041,24 @@ SELECT madlib.validation_preprocessor_dl('iris_test',
-- Source table
SELECT * FROM iris_test_packed_summary;
</pre>
<pre class="result">
--[ RECORD 1 ]-------+---------------------------------------------
-source_table | iris_test
-output_table | iris_test_packed
-dependent_varname | class_text
-independent_varname | attributes
-dependent_vartype | character varying
-class_values | {Iris-setosa,Iris-versicolor,Iris-virginica}
-buffer_size | 15
-normalizing_const | 1.0
-num_classes | 3
+-[ RECORD 1 ]-----------+---------------------------------------------
+source_table | iris_test
+output_table | iris_test_packed
+dependent_varname | {class_text}
+independent_varname | {attributes}
+dependent_vartype | {"character varying"}
+class_text_class_values | {Iris-setosa,Iris-versicolor,Iris-virginica}
+buffer_size | 10
+normalizing_const | 1
+num_classes | {3}
+distribution_rules | all_segments
+__internal_gpu_config__ | all_segments
</pre>
-# Define and load model architecture. Use Keras to define
the model architecture:
<pre class="example">
-import keras
+from tensorflow import keras
from keras.models import Sequential
Review comment:
we should also update the next two lines to use tensorflow.keras
##########
File path:
src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -20,7 +20,7 @@
*
* @file model_arch_table.sql_in
*
- * @brief SQL functions for multilayer perceptron
+ * @brief Function to load model architectures and weights into a table.
* @date June 2012
Review comment:
should we update the date as well ?
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras.py_in
##########
@@ -430,6 +431,7 @@ def fit(schema_madlib, source_table, model,
model_arch_table,
ARRAY{dep_vartype}::TEXT[] AS {dependent_vartype_colname},
{norm_const}::{FLOAT32_SQL_TYPE} AS {normalizing_const_colname},
{metrics_type}::TEXT[] AS metrics_type,
+ '{loss_type}'::TEXT AS loss_type,
Review comment:
we can add an assertion in our dev check tests that loss_type was set to
the expected value.
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras.sql_in
##########
@@ -178,6 +188,11 @@ madlib_keras_fit(
There are no mandatory parameters so
if you specify NULL, it will use all default
values as per Keras.
+
+ @note
+ Callbacks are not currently supported except for TensorBoard
Review comment:
maybe we should also add a note to mention all the supported tensorboard
params
##########
File path:
src/ports/postgres/modules/deep_learning/madlib_keras_model_selection.sql_in
##########
@@ -288,7 +302,7 @@ load_model_selection_table(
so we first create a model architecture table with two different models. Use
Keras to define
a model architecture with 1 hidden layer:
<pre class="example">
-import keras
+from tensorflow import keras
Review comment:
we should use tensorflow.keras in the next two lines as well
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_automl.sql_in
##########
@@ -900,7 +923,7 @@ num_classes | 3
-# Define and load model architecture. Use Keras to define
the model architecture with 1 hidden layer:
<pre class="example">
-import keras
+from tensorflow import keras
Review comment:
we should use tensorflow.keras in the next two lines as well
##########
File path:
src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -42,16 +39,25 @@ Interface and implementation are subject to change. </em>
<li class="level1"><a href="#related">Related Topics</a></li>
</ul></div>
-This utility function loads model architectures and
+This function loads model architectures and
weights into a table for use by deep learning algorithms.
+
Model architecture is in JSON form
and model weights are in the form of PostgreSQL binary data types (bytea).
If the output table already exists, a new row is inserted
into the table so it can act as a repository for multiple model
architectures and weights.
-There is also a utility function to delete a model
-from the table.
+There is also a function to delete a model from the table.
+
+MADlib's deep learning methods are designed to use the TensorFlow package and
its built in Keras
+functions. To ensure consistency, please use tensorflow.keras objects
(models, layers, etc.)
+instead of importing Keras and using its objects.
+
+Please note that the first <em>n</em> layers of the model must have the shape
Review comment:
I think we only need to add the input shape to the first layer
##########
File path:
src/ports/postgres/modules/deep_learning/madlib_keras_fit_multiple_model.sql_in
##########
@@ -879,7 +914,7 @@ num_classes | 3
-# Define and load model architecture. Use Keras to define
the model architecture with 1 hidden layer:
<pre class="example">
-import keras
+from tensorflow import keras
Review comment:
we should use tensorflow.keras in the next two lines as well
----------------------------------------------------------------
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:
us...@infra.apache.org
