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