You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by GitBox <gi...@apache.org> on 2021/01/29 17:57:21 UTC
[GitHub] [madlib] orhankislal opened a new pull request #534: DL: Various docs changes
orhankislal opened a new pull request #534:
URL: https://github.com/apache/madlib/pull/534
<!--
Thanks for sending a pull request! Here are some tips for you:
1. Refer to this link for contribution guidelines https://cwiki.apache.org/confluence/display/MADLIB/Contribution+Guidelines
2. Please Provide the Module Name, a JIRA Number and a short description about your changes.
-->
- [ ] Add the module name, JIRA# to PR/commit and description.
- [ ] Add tests for the change.
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r585809058
##########
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:
added this to user docs
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583891985
##########
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:
fixed
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on pull request #534:
URL: https://github.com/apache/madlib/pull/534#issuecomment-783803053
@orhankislal
missing 'loss_type' in output table from
.../workspace/madlib/build_docker/doc/user/html/group__grp__keras.html#keras_fit
I think we missed this in
https://github.com/apache/madlib/pull/511
where we added to info table for multi_fit() but forgot fit()
can you add it in please? thx
```
-[ RECORD 1 ]-------------+--------------------------------------------------------------------------
source_table | iris_train_packed
model | iris_model
dependent_varname | {class_text}
independent_varname | {attributes}
model_arch_table | model_arch_library
model_id | 1
compile_params | loss='categorical_crossentropy', optimizer='adam', metrics=['accuracy']
fit_params | batch_size=5, epochs=3
num_iterations | 10
validation_table |
object_table |
metrics_compute_frequency | 10
name |
description |
model_type | madlib_keras
model_size | 0.7900390625
start_training_time | 2021-02-22 22:39:59.223927
end_training_time | 2021-02-22 22:40:02.695402
metrics_elapsed_time | {3.47141289710999}
madlib_version | 1.18.0-dev
num_classes | {3}
dependent_vartype | {"character varying"}
normalizing_const | 1
metrics_type | {accuracy}
loss_type | xxxyyyzzz <- **** missing ****
training_metrics_final | 0.833333313465118
training_loss_final | 0.745647132396698
training_metrics | {0.833333313465118}
training_loss | {0.745647132396698}
validation_metrics_final |
validation_loss_final |
validation_metrics |
validation_loss |
metrics_iters | {10}
class_text_class_values | {Iris-setosa,Iris-versicolor,Iris-virginica}
```
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
orhankislal commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r585485332
##########
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:
To use multiple inputs, the model has to have the same number of input layers.
Example:
```
input1 = Input(shape = (4,))
input2 = Input(shape = (4,))
dense1_1 = Dense(10, activation='relu', kernel_initializer=VarianceScaling(distribution="uniform"))(input1)
dense2_1 = Dense(10, activation='relu', kernel_initializer=VarianceScaling(distribution="uniform"))(input2)
concat = concatenate([dense1_1,dense2_1])
...
```
In keras, it is possible to create an input layer without the input_shape; however, MADlib cannot work with those models and we have to let the user know that their model definition has to have this parameter. Note that this restriction was already in place, prior to the implementation of multi-io.
Please refer to the dev-check tests for a complete model example and model_arch_info.py_in line 28 for how this information is used.
Here is my suggestion for the docs:
```
Please note that every input layer has to have the `input_shape` stated explicitly during model creation. MADlib has this requirement because, in some cases, the JSON representation may not have the input shape by default and it has to be read from the JSON for fit() type functions.
```
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583890061
##########
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:
do you mean from https://www.tensorflow.org/api_docs/python/tf/keras/callbacks/TensorBoard ? @kaknikhil I am not sure which params we support and do not support, do you?
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 edited a comment on pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 edited a comment on pull request #534:
URL: https://github.com/apache/madlib/pull/534#issuecomment-784564784
yes looks good now with `loss_type` in output
```SELECT * FROM iris_model_summary;
-[ RECORD 1 ]-------------+--------------------------------------------------------------------------
source_table | iris_train_packed
model | iris_model
dependent_varname | {class_text}
independent_varname | {attributes}
model_arch_table | model_arch_library
model_id | 1
compile_params | loss='categorical_crossentropy', optimizer='adam', metrics=['accuracy']
fit_params | batch_size=5, epochs=3
num_iterations | 10
validation_table |
object_table |
metrics_compute_frequency | 10
name |
description |
model_type | madlib_keras
model_size | 0.7900390625
start_training_time | 2021-02-23 22:33:45.814856
end_training_time | 2021-02-23 22:33:49.155229
metrics_elapsed_time | {3.34029412269592}
madlib_version | 1.18.0-dev
num_classes | {3}
dependent_vartype | {"character varying"}
normalizing_const | 1
metrics_type | {accuracy}
loss_type | categorical_crossentropy
training_metrics_final | 0.941666662693024
training_loss_final | 0.362120628356934
training_metrics | {0.941666662693024}
training_loss | {0.362120628356934}
validation_metrics_final |
validation_loss_final |
validation_metrics |
validation_loss |
metrics_iters | {10}
class_text_class_values | {Iris-setosa,Iris-versicolor,Iris-virginica}
```
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583964864
##########
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:
i'd suggest we don't spend anymore time on looking at this, there may be TF1.x vs 2.x diffs plus we plan to look at callbacks in the next release 1.19.0 in a more systematic way. So I am OK with it as is.
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on pull request #534:
URL: https://github.com/apache/madlib/pull/534#issuecomment-785337568
@kaknikhil Can you please review? Focus less on user doc content and more on software changes, thx
----------------------------------------------------------------
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
[GitHub] [madlib] kaknikhil commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [madlib] orhankislal commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
orhankislal commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r570125221
##########
File path: src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -333,7 +333,7 @@ SELECT COUNT(*) FROM model_arch_library;
<pre class="result">
count
-------+
- 1
+ 2
Review comment:
Oh I see, thanks for pointing it out.
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
orhankislal commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r570125221
##########
File path: src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -333,7 +333,7 @@ SELECT COUNT(*) FROM model_arch_library;
<pre class="result">
count
-------+
- 1
+ 2
Review comment:
Oh I see, thanks for pointing it out.
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal merged pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
orhankislal merged pull request #534:
URL: https://github.com/apache/madlib/pull/534
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on pull request #534:
URL: https://github.com/apache/madlib/pull/534#issuecomment-789121252
LGTM
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583890902
##########
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:
fixed
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583892982
##########
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:
fixed
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583893241
##########
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:
fixed
----------------------------------------------------------------
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
[GitHub] [madlib] kaknikhil commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
kaknikhil commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583902808
##########
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:
Looking at the code, here is what we support
```
'log_dir', 'histogram_freq', 'batch_size', 'update_freq', 'write_graph', 'write_grad', 'write_images'
```
@orhankislal or @reductionista Can you confirm ?
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
orhankislal commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r569345185
##########
File path: src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -333,7 +333,7 @@ SELECT COUNT(*) FROM model_arch_library;
<pre class="result">
count
-------+
- 1
+ 2
Review comment:
This should be 1 since we delete a model out of 2
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r569888333
##########
File path: src/ports/postgres/modules/deep_learning/keras_model_arch_table.sql_in
##########
@@ -333,7 +333,7 @@ SELECT COUNT(*) FROM model_arch_library;
<pre class="result">
count
-------+
- 1
+ 2
Review comment:
we delete 1 model out of 3 actually, so 2 is correct.
the query just above in step 3 is
```
SELECT COUNT(*) FROM model_arch_library WHERE model_weights IS NOT NULL;
```
which just counts models that have non-null wts (there are 2), not the total list of models (there are 3 at that point)
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583889431
##########
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:
fixed date
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583890061
##########
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:
do you mean from https://www.tensorflow.org/api_docs/python/tf/keras/callbacks/TensorBoard ? @kaknikhil
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583964864
##########
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:
i'd suggest we don't spend anymore time on looking at this, there may be TF1.x vs 2.x diffs plus we plan to look at callbacks in the next release 1.19.0 in a more systematic way
----------------------------------------------------------------
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
[GitHub] [madlib] kaknikhil commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
kaknikhil commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583864575
##########
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 but let me check again and I will update this comment
----------------------------------------------------------------
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
[GitHub] [madlib] kaknikhil commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
kaknikhil commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r583867354
##########
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:
Just to be more explicit, we should use tensorflow.keras in the next two lines as well
##########
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:
Just to be more explicit, 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:
Just to be more explicit, we should use tensorflow.keras in the next two lines as well
##########
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:
Just to be more explicit, 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:
users@infra.apache.org
[GitHub] [madlib] fmcquillan99 commented on a change in pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on a change in pull request #534:
URL: https://github.com/apache/madlib/pull/534#discussion_r584932875
##########
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:
@orhankislal ^^^ please clarify and suggest wording on this
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on pull request #534: DL: Various docs changes
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on pull request #534:
URL: https://github.com/apache/madlib/pull/534#issuecomment-784564784
yes looks good now with `loss_type` in summary output
```SELECT * FROM iris_model_summary;
-[ RECORD 1 ]-------------+--------------------------------------------------------------------------
source_table | iris_train_packed
model | iris_model
dependent_varname | {class_text}
independent_varname | {attributes}
model_arch_table | model_arch_library
model_id | 1
compile_params | loss='categorical_crossentropy', optimizer='adam', metrics=['accuracy']
fit_params | batch_size=5, epochs=3
num_iterations | 10
validation_table |
object_table |
metrics_compute_frequency | 10
name |
description |
model_type | madlib_keras
model_size | 0.7900390625
start_training_time | 2021-02-23 22:33:45.814856
end_training_time | 2021-02-23 22:33:49.155229
metrics_elapsed_time | {3.34029412269592}
madlib_version | 1.18.0-dev
num_classes | {3}
dependent_vartype | {"character varying"}
normalizing_const | 1
metrics_type | {accuracy}
loss_type | categorical_crossentropy
training_metrics_final | 0.941666662693024
training_loss_final | 0.362120628356934
training_metrics | {0.941666662693024}
training_loss | {0.362120628356934}
validation_metrics_final |
validation_loss_final |
validation_metrics |
validation_loss |
metrics_iters | {10}
class_text_class_values | {Iris-setosa,Iris-versicolor,Iris-virginica}
```
----------------------------------------------------------------
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