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



##########
File path: 
src/ports/postgres/modules/deep_learning/test/madlib_keras_automl.sql_in
##########
@@ -329,7 +330,8 @@ DROP TABLE IF EXISTS automl_output, automl_output_info, 
automl_output_summary, a
 SELECT madlib_keras_automl('iris_data_packed', 'automl_output', 
'iris_model_arch', 'automl_mst_table',
     ARRAY[1,2], $${'loss': ['categorical_crossentropy'], 
'optimizer_params_list': [ {'optimizer': ['Adagrad', 'Adam'],
     'lr': [0.9, 0.95, 'log'], 'epsilon': [0.3, 0.5, 'log_near_one']}, 
{'optimizer': ['Adam', 'SGD'],
-    'lr': [0.6, 0.65, 'log']} ], 'metrics':['accuracy'] }$$, $${'batch_size': 
[2, 4], 'epochs': [3]}$$);
+    'lr': [0.6, 0.65, 'log']} ], 'metrics':['accuracy'] }$$,
+    $${'batch_size': [2, 4], 'epochs': [3], 'callbacks': 
['[TensorBoard(log_dir=\'/tmp/tensorflow/scalars/\')]']}$$);

Review comment:
       The syntax for passing these AutoML params is getting pretty ugly.
   
   Part of the reason is the use of \\' instead of ", which appears to be a 
workaround for this bug:
   https://issues.apache.org/jira/projects/MADLIB/issues/MADLIB-1455
   
   This might be a good time to address that one, I would guess it's pretty low 
effort.  With double quotes working, it would look more like typical python:
   
   `{ 'epochs' : [3], 'callbacks': [ 
'[TensorBoard(log_dir="/tmp/tensorflow/scalars/"]' ], 'batch_size' : [2, 4] }`
   
   Eventually, if we add support for arbitrary custom callbacks, we should 
assess whether there is a better way for the user to pass an array of different 
callback choices they want AutoML to combine with different models.
   
   For now, since we only support TensorBoard() and it's just used for 
monitoring/debugging... it's probably safe to assume they'll always want the 
same Tensorboard callback for every model... rather than multiple choices of 
callbacks.
   
   One thing that might improve it further though, is allowing the [] to be 
optional for compile/fit params which only have a single choice, like this:
   
   ` 'epochs' : 3`   means the same as =>  `'epochs' : [3]`
   and
   `'callbacks' : '[ TensorBoard(log_dir="/tmp/tensorflow/scalars/") ]'`
        == means the same as ==>  `'callbacks' :  [ '[ 
TensorBoard(log_dir="/tmp/tensorflow/scalars/") ]' ]`




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


Reply via email to