khannaekta commented on a change in pull request #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#discussion_r362939980
##########
File path:
src/ports/postgres/modules/deep_learning/madlib_keras_validator.py_in
##########
@@ -437,13 +437,6 @@ class FitMultipleInputValidator(FitCommonValidator):
accessible_gpus_for_seg,
self.module_name)
- if warm_start:
- mst_count = plpy.execute("SELECT count(*) FROM
{0}".format(model_selection_table))[0]['count']
- warm_count = plpy.execute("SELECT count(*) FROM
{0}".format(output_model_table))[0]['count']
-
- _assert(mst_count <= warm_count,
- "{self.module_name} error: Model table and mst table do not
match".format(self=self))
Review comment:
This should have failed the test for warm start with extra mst
keys(`madlib_keras_transfer_learning.sql_in:L240`).
Spent some time trying to figure why the test did not fail: It is because
the test was missing a `SELECT` when calling the trap_error function for the
test.
Instead of the following:
```
SELECT assert(trap_error($TRAP$ madlib_keras_fit_multiple_model(
'iris_data_packed',
'iris_multiple_model',
'mst_table',
3,
FALSE,
NULL, 1,
TRUE -- warm_start
);$TRAP$) = 1, 'Warm start with extra mst keys should fail.');
```
It should have been:
```
SELECT assert(trap_error($TRAP$ SELECT madlib_keras_fit_multiple_model(
'iris_data_packed',
'iris_multiple_model',
'mst_table',
3,
FALSE,
NULL, 1,
TRUE -- warm_start
);$TRAP$) = 1, 'Warm start with extra mst keys should fail.');
```
With this code change, we should update the test for warm start with extra
mst keys(`madlib_keras_transfer_learning.sql_in:L240`) to not throw any error.
----------------------------------------------------------------
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
