[GitHub] [madlib] khannaekta commented on a change in pull request #519: DL: Add a helper function to load custom top n accuracy functions

[GitBox <gi...@apache.org>]

khannaekta commented on a change in pull request #519:
URL: https://github.com/apache/madlib/pull/519#discussion_r498537268



##########
File path: src/ports/postgres/modules/deep_learning/test/madlib_keras_custom_function.sql_in
##########
@@ -31,107 +31,130 @@ m4_include(`SQLCommon.m4')
 )
 
 /* Test successful table creation where no table exists */
-DROP TABLE IF EXISTS test_custom_function_table;
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', 'returns sum');
+DROP TABLE IF EXISTS __test_custom_function_table__;
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', 'returns sum');
 
 SELECT assert(UPPER(atttypid::regtype::TEXT) = 'INTEGER', 'id column should be INTEGER type')
-    FROM pg_attribute WHERE attrelid = 'test_custom_function_table'::regclass
+    FROM pg_attribute WHERE attrelid = '__test_custom_function_table__'::regclass
         AND attname = 'id';
 SELECT assert(UPPER(atttypid::regtype::TEXT) = 'BYTEA', 'object column should be BYTEA type' )
-    FROM pg_attribute WHERE attrelid = 'test_custom_function_table'::regclass
+    FROM pg_attribute WHERE attrelid = '__test_custom_function_table__'::regclass
         AND attname = 'object';
 SELECT assert(UPPER(atttypid::regtype::TEXT) = 'TEXT',
     'name column should be TEXT type')
-    FROM pg_attribute WHERE attrelid = 'test_custom_function_table'::regclass
+    FROM pg_attribute WHERE attrelid = '__test_custom_function_table__'::regclass
         AND attname = 'name';
 SELECT assert(UPPER(atttypid::regtype::TEXT) = 'TEXT',
     'description column should be TEXT type')
-    FROM pg_attribute WHERE attrelid = 'test_custom_function_table'::regclass
+    FROM pg_attribute WHERE attrelid = '__test_custom_function_table__'::regclass
         AND attname = 'description';
 
 /*  id should be 1 */
 SELECT assert(id = 1, 'Wrong id written by load_custom_function')
-    FROM test_custom_function_table;
+    FROM __test_custom_function_table__;
 
 /* Validate function object created */
 SELECT assert(read_custom_function(object, 2, 3) = 5, 'Custom function should return sum of args.')
-    FROM test_custom_function_table;
+    FROM __test_custom_function_table__;
 
 /* Test custom function insertion where valid table exists */
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn1');
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn1');
 SELECT assert(name = 'sum_fn', 'Custom function sum_fn found in table.')
-    FROM test_custom_function_table WHERE id = 1;
+    FROM __test_custom_function_table__ WHERE id = 1;
 SELECT assert(name = 'sum_fn1', 'Custom function sum_fn1 found in table.')
-    FROM test_custom_function_table WHERE id = 2;
+    FROM __test_custom_function_table__ WHERE id = 2;
 
 /* Test adding an existing function name should error out */
 SELECT assert(MADLIB_SCHEMA.trap_error($TRAP$
-    SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn1');
+    SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn1');
     $TRAP$) = 1, 'Should error out for duplicate function name');
 
 /* Test deletion by id where valid table exists */
 /* Assert id exists before deleting */
 SELECT assert(COUNT(id) = 1, 'id 2 should exist before deletion!')
-    FROM test_custom_function_table WHERE id = 2;
-SELECT delete_custom_function('test_custom_function_table', 2);
+    FROM __test_custom_function_table__ WHERE id = 2;
+SELECT delete_custom_function('__test_custom_function_table__', 2);
 SELECT assert(COUNT(id) = 0, 'id 2 should have been deleted!')
-    FROM test_custom_function_table WHERE id = 2;
+    FROM __test_custom_function_table__ WHERE id = 2;
 
 /* Test deletion by name where valid table exists */
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn1');
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn1');
 /* Assert id exists before deleting */
 SELECT assert(COUNT(id) = 1, 'function name sum_fn1 should exist before deletion!')
-    FROM test_custom_function_table WHERE name = 'sum_fn1';
-SELECT delete_custom_function('test_custom_function_table', 'sum_fn1');
+    FROM __test_custom_function_table__ WHERE name = 'sum_fn1';
+SELECT delete_custom_function('__test_custom_function_table__', 'sum_fn1');
 SELECT assert(COUNT(id) = 0, 'function name sum_fn1 should have been deleted!')
-    FROM test_custom_function_table WHERE name = 'sum_fn1';
+    FROM __test_custom_function_table__ WHERE name = 'sum_fn1';
 
 /* Test deleting an already deleted entry should error out */
 SELECT assert(MADLIB_SCHEMA.trap_error($TRAP$
-    SELECT delete_custom_function('test_custom_function_table', 2);
+    SELECT delete_custom_function('__test_custom_function_table__', 2);
     $TRAP$) = 1, 'Should error out for trying to delete an entry that does not exist');
 
 /* Test delete drops the table after deleting last entry*/
-DROP TABLE IF EXISTS test_custom_function_table;
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', 'returns sum');
-SELECT delete_custom_function('test_custom_function_table', 1);
-SELECT assert(COUNT(relname) = 0, 'Table test_custom_function_table should have been deleted.')
-    FROM pg_class where relname='test_custom_function_table';
+DROP TABLE IF EXISTS __test_custom_function_table__;
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', 'returns sum');
+SELECT delete_custom_function('__test_custom_function_table__', 1);
+SELECT assert(COUNT(relname) = 0, 'Table __test_custom_function_table__ should have been deleted.')
+    FROM pg_class where relname='__test_custom_function_table__';
 
 /* Test deletion where empty table exists */
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', 'returns sum');
-DELETE FROM test_custom_function_table;
-SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('test_custom_function_table', 1)$$) = 1,
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', 'returns sum');
+DELETE FROM __test_custom_function_table__;
+SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('__test_custom_function_table__', 1)$$) = 1,
     'Deleting function in an empty table should generate an exception.');
 
 /* Test deletion where no table exists */
-DROP TABLE IF EXISTS test_custom_function_table;
-SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('test_custom_function_table', 1)$$) = 1,
+DROP TABLE IF EXISTS __test_custom_function_table__;
+SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('__test_custom_function_table__', 1)$$) = 1,
               'Deleting a non-existent table should raise exception.');
 
 /* Test where invalid table exists */
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', 'returns sum');
-ALTER TABLE test_custom_function_table DROP COLUMN id;
-SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('test_custom_function_table', 2)$$) = 1,
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', 'returns sum');
+ALTER TABLE __test_custom_function_table__ DROP COLUMN id;
+SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT delete_custom_function('__test_custom_function_table__', 2)$$) = 1,
     'Deleting an invalid table should generate an exception.');
 
-SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', 'returns sum')$$) = 1,
+SELECT assert(MADLIB_SCHEMA.trap_error($$SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', 'returns sum')$$) = 1,
     'Passing an invalid table to load_custom_function() should raise exception.');
 
 /* Test input validation */
-DROP TABLE IF EXISTS test_custom_function_table;
+DROP TABLE IF EXISTS __test_custom_function_table__;
 SELECT assert(MADLIB_SCHEMA.trap_error($$
-  SELECT load_custom_function('test_custom_function_table', custom_function_object(), NULL, NULL);
+  SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), NULL, NULL);
 $$) = 1, 'Name cannot be NULL');
 SELECT assert(MADLIB_SCHEMA.trap_error($$
-  SELECT load_custom_function('test_custom_function_table', NULL, 'sum_fn', NULL);
+  SELECT load_custom_function('__test_custom_function_table__', NULL, 'sum_fn', NULL);
 $$) = 1, 'Function object cannot be NULL');
 SELECT assert(MADLIB_SCHEMA.trap_error($$
-  SELECT load_custom_function('test_custom_function_table', 'invalid_obj'::bytea, 'sum_fn', NULL);
+  SELECT load_custom_function('__test_custom_function_table__', 'invalid_obj'::bytea, 'sum_fn', NULL);
 $$) = 1, 'Invalid custom function object');
-SELECT load_custom_function('test_custom_function_table', custom_function_object(), 'sum_fn', NULL);
+SELECT load_custom_function('__test_custom_function_table__', custom_function_object(), 'sum_fn', NULL);
 SELECT assert(name IS NOT NULL AND description IS NULL, 'validate name is not NULL.')
-    FROM test_custom_function_table;
+    FROM __test_custom_function_table__;
 SELECT assert(MADLIB_SCHEMA.trap_error($$
-  SELECT delete_custom_function('test_custom_function_table', NULL);
+  SELECT delete_custom_function('__test_custom_function_table__', NULL);
 $$) = 1, 'id/name cannot be NULL!');
+
+/* Test top n accuracy */
+
+DROP TABLE IF EXISTS __test_custom_function_table__;
+SELECT load_top_n_accuracy_function('__test_custom_function_table__', 3);
+SELECT load_top_n_accuracy_function('__test_custom_function_table__', 7);
+SELECT load_top_n_accuracy_function('__test_custom_function_table__', 7, True);

Review comment:
       I see that it prints the following INFO when executing the function :
   ```
   INFO:  Keras Custom Function: Added function sparse_top_7_accuracy to __test_custom_function_table__ table
   CONTEXT:  PL/Python function "load_custom_function"
   SQL statement "
           SELECT  madlib.load_custom_function('__test_custom_function_table__',
                   __madlib__sparse_top_7_acc_pickled(),
                   'sparse_top_7_accuracy',
                   'returns sparse_top_7_accuracy');
   ```
   
   This comes from the call to `load_custom_function()`. Looking at the above output was just thinking if we should print that out in load_custom_function. Thoughts ?
   

##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_custom_function.py_in
##########
@@ -146,6 +146,43 @@ def update_builtin_metrics(builtin_metrics):
     builtin_metrics.append('ce')
     return builtin_metrics
 
+@MinWarning("error")
+def load_top_n_accuracy_function(schema_madlib, object_table, n, is_sparse, **kwargs):
+
+    object_table = quote_ident(object_table)
+    _assert(n > 0,
+        "{0}: For top n accuracy functions n has to be a positive integer.".format(module_name))
+    sparse_prefix = "sparse_" if is_sparse else ""
+
+    sql = """
+    CREATE OR REPLACE FUNCTION __madlib__{sparse_prefix}top_{n}_acc_pickled()

Review comment:
       Will the user be constraint from using this utility based on their privileges ? Since we are trying to create a function inside the utility, if a user doesn't have privileges to create UDF's, I suppose this will error 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