reductionista commented on a change in pull request #467: DL: Improve performance of mini-batch preprocessor URL: https://github.com/apache/madlib/pull/467#discussion_r362999239
########## File path: src/ports/postgres/modules/deep_learning/input_data_preprocessor.py_in ########## @@ -46,9 +48,25 @@ from utilities.validate_args import input_tbl_valid from utilities.validate_args import get_expr_type from madlib_keras_helper import * +import time NUM_CLASSES_COLNAME = "num_classes" +def plpy_execute(sql, *args, **kwargs): Review comment: Yes, I like the idea of moving to utilities--could be useful for other modules as well. Naming the function `plpy_execute_debug()` and keeping the code as-is would give the false impression that calling this instead of `plpy.execute()` turns on debugging. In any fully-committed version of the code, it will behave identically to `plpy.execute()`. Only by adding the `debug=True` option can debugging be turned on. Intended use case is to be able to add detailed debugging to any sql query in a dev branch with a quick temporary change, instead of copying boiler plate around. So, as currently written, someone would change: `plpy_execute(sql)` to: `plpy_execute(sql, debug=True)` in each place they want debugging on. But we could change the function so instead of the above workflow, it looks like this: `plpy.execute(sql)` `plpy_execute_debug(sql)` So I'll change the default from False to True and then change all instances of `plpy_execute` back to `plpy.execute`. I think that will solve the problem and accomplish roughly what you suggested. ---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
