You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by GitBox <gi...@apache.org> on 2019/12/20 23:14:42 UTC
[GitHub] [madlib] orhankislal opened a new pull request #466: DL: Modify
multi-fit warm start to accept non-matching mst&model tables
orhankislal opened a new pull request #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466
JIRA: MADLIB-1400
The warm start enforced that the model table had to have a tuple for each
mst_key in the mst table for warm start. This commit relaxes this requirement
so that users can add as well as substract mst keys throughtout their
AutoML progress.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. Refer to this link for contribution guidelines https://cwiki.apache.org/confluence/display/MADLIB/Contribution+Guidelines
2. Please Provide the Module Name, a JIRA Number and a short description about your changes.
-->
- [ ] Add the module name, JIRA# to PR/commit and description.
- [ ] Add tests for the change.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] khannaekta commented on a change in pull request #466: DL:
Modify multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
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_r362956766
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_fit_multiple_model.py_in
##########
@@ -274,65 +274,66 @@ class FitMultipleModel():
plpy.execute(mst_insert_query)
def create_model_output_table(self):
- output_table_create_query = """
- CREATE TABLE {self.model_output_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_weights_col} BYTEA,
- {self.model_arch_col} JSON)
- """.format(self=self)
- info_table_create_query = """
- CREATE TABLE {self.model_info_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_id_col} INTEGER,
- {self.compile_params_col} TEXT,
- {self.fit_params_col} TEXT,
- model_type TEXT,
- model_size DOUBLE PRECISION,
- metrics_elapsed_time DOUBLE PRECISION[],
- metrics_type TEXT[],
- training_metrics_final DOUBLE PRECISION,
- training_loss_final DOUBLE PRECISION,
- training_metrics DOUBLE PRECISION[],
- training_loss DOUBLE PRECISION[],
- validation_metrics_final DOUBLE PRECISION,
- validation_loss_final DOUBLE PRECISION,
- validation_metrics DOUBLE PRECISION[],
- validation_loss DOUBLE PRECISION[])
- """.format(self=self)
-
- plpy.execute(output_table_create_query)
- plpy.execute(info_table_create_query)
+ warm_start_msts = []
+ if self.warm_start:
+ warm_start_msts = plpy.execute(
+ """ SELECT array_agg({0}) AS a FROM {1}
+ """.format(self.mst_key_col, self.model_output_table))[0]['a']
+ plpy.execute("TRUNCATE TABLE {0}".format(self.model_info_table))
Review comment:
We shouldn't be Truncating the info table. As for the following scenario,
- mst_table has 4 mst_keys
- run `madlib_keras_fit_multiple_model()` -> output and info table has 4 mst_key values
- Delete 1 mst_key, run `madlib_keras_fit_multiple_model()` with warm_start = True
With this PR:
output table has 4 mst_key values and info table has only 3 mst_key
On master:
output table and info table has 4 mst_key
We should also update the test `madlib_keras_transfer_learning.sql_in:228` to Fail for the above scenario. Currently, it passes as the assert query returns 0 rows.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 edited a comment on issue #466: DL: Modify
multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 edited a comment on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571779179
Some more testing, seems to pick up from where it left off when warm start is on, though these curves are somewhat flat.
```
SELECT validation_loss, validation_metrics FROM results_mnist where s=2 AND mst_key = 8;
```
results in
```
-[ RECORD 1 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0454168096184731}
validation_metrics | {0.986400008201599}
-[ RECORD 2 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0372964888811111,0.0447742976248264,0.0334407761693001}
validation_metrics | {0.98769998550415,0.986100018024445,0.989400029182434}
-[ RECORD 3 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0316416546702385,0.0328738279640675,0.0300650335848331,0.0358279161155224,0.0323219522833824,0.0301002375781536,0.0322590619325638,0.0325789600610733,0.0320237688720226}
validation_metrics | {0.98989999294281,0.989600002765656,0.990899980068207,0.988699972629547,0.991400003433228,0.991999983787537,0.989700019359589,0.990400016307831,0.990899980068207}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] khannaekta commented on a change in pull request #466: DL:
Modify multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
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_r362944135
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_fit_multiple_model.py_in
##########
@@ -274,65 +274,66 @@ class FitMultipleModel():
plpy.execute(mst_insert_query)
def create_model_output_table(self):
- output_table_create_query = """
- CREATE TABLE {self.model_output_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_weights_col} BYTEA,
- {self.model_arch_col} JSON)
- """.format(self=self)
- info_table_create_query = """
- CREATE TABLE {self.model_info_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_id_col} INTEGER,
- {self.compile_params_col} TEXT,
- {self.fit_params_col} TEXT,
- model_type TEXT,
- model_size DOUBLE PRECISION,
- metrics_elapsed_time DOUBLE PRECISION[],
- metrics_type TEXT[],
- training_metrics_final DOUBLE PRECISION,
- training_loss_final DOUBLE PRECISION,
- training_metrics DOUBLE PRECISION[],
- training_loss DOUBLE PRECISION[],
- validation_metrics_final DOUBLE PRECISION,
- validation_loss_final DOUBLE PRECISION,
- validation_metrics DOUBLE PRECISION[],
- validation_loss DOUBLE PRECISION[])
- """.format(self=self)
-
- plpy.execute(output_table_create_query)
- plpy.execute(info_table_create_query)
+ warm_start_msts = []
+ if self.warm_start:
+ warm_start_msts = plpy.execute(
+ """ SELECT array_agg({0}) AS a FROM {1}
+ """.format(self.mst_key_col, self.model_output_table))[0]['a']
+ plpy.execute("TRUNCATE TABLE {0}".format(self.model_info_table))
+ else:
+ output_table_create_query = """
+ CREATE TABLE {self.model_output_table}
+ ({self.mst_key_col} INTEGER PRIMARY KEY,
+ {self.model_weights_col} BYTEA,
+ {self.model_arch_col} JSON)
+ """.format(self=self)
+ info_table_create_query = """
+ CREATE TABLE {self.model_info_table}
+ ({self.mst_key_col} INTEGER PRIMARY KEY,
+ {self.model_id_col} INTEGER,
+ {self.compile_params_col} TEXT,
+ {self.fit_params_col} TEXT,
+ model_type TEXT,
+ model_size DOUBLE PRECISION,
+ metrics_elapsed_time DOUBLE PRECISION[],
+ metrics_type TEXT[],
+ training_metrics_final DOUBLE PRECISION,
+ training_loss_final DOUBLE PRECISION,
+ training_metrics DOUBLE PRECISION[],
+ training_loss DOUBLE PRECISION[],
+ validation_metrics_final DOUBLE PRECISION,
+ validation_loss_final DOUBLE PRECISION,
+ validation_metrics DOUBLE PRECISION[],
+ validation_loss DOUBLE PRECISION[])
+ """.format(self=self)
+
+ plpy.execute(output_table_create_query)
+ plpy.execute(info_table_create_query)
for mst in self.msts:
model_arch, model_weights = get_model_arch_weights(self.model_arch_table,
mst[self.model_id_col])
+
+
+ # If warm start is enabled, weigths from transfer learning cannot be
Review comment:
typo `weigths`-> `weights`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 commented on issue #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571779179
Some more testing, seems to pick up from where it left off when warm start is on.
```
SELECT validation_loss, validation_metrics FROM results_mnist where s=2 AND mst_key = 8;
```
results in
```
-[ RECORD 1 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0454168096184731}
validation_metrics | {0.986400008201599}
-[ RECORD 2 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0372964888811111,0.0447742976248264,0.0334407761693001}
validation_metrics | {0.98769998550415,0.986100018024445,0.989400029182434}
-[ RECORD 3 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0316416546702385,0.0328738279640675,0.0300650335848331,0.0358279161155224,0.0323219522833824,0.0301002375781536,0.0322590619325638,0.0325789600610733,0.0320237688720226}
validation_metrics | {0.98989999294281,0.989600002765656,0.990899980068207,0.988699972629547,0.991400003433228,0.991999983787537,0.989700019359589,0.990400016307831,0.990899980068207}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] orhankislal commented on issue #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
orhankislal commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571837636
@fmcquillan99 Regarding the table lock: I am actually not that surprised to see the lock on the `info` table since this is a single sql query and `delete`/`truncate` operations do enforce `access exclusive` lock. I would like to understand why the behavior is different for the summary table. We should explore it at some point but I don't see it as a bug of this implementation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 commented on issue #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571377290
The warm start logic seems OK in limited testing so far, but during autoML runs, I cannot query the `_info` table after it has been created because it has an `AccessExclusiveLock`. I can however query the `_summary` table. Not sure why it would have this lock? I re-started the database and tried again but same thing:
```
madlib=# select t.relname,l.locktype,page,virtualtransaction,pid,mode,granted from pg_locks l, pg_stat_all_tables t where l.relation=t.relid order by relation asc;
relname | locktype | page | virtualtransaction | pid | mode | granted
---------------------------------+----------+------+--------------------+-------+---------------------+---------
train_mnist_packed | relation | | 4/122 | 28078 | AccessShareLock | t
train_mnist_packed | relation | | 1/164 | 28091 | AccessShareLock | t
train_mnist_packed | relation | | 1/164 | 28092 | AccessShareLock | t
train_mnist_packed_summary | relation | | 4/122 | 28078 | AccessShareLock | t
train_mnist_packed_summary | relation | | 1/164 | 28091 | AccessShareLock | t
train_mnist_packed_summary | relation | | 1/164 | 28092 | AccessShareLock | t
test_mnist_packed | relation | | 1/164 | 28091 | AccessShareLock | t
test_mnist_packed | relation | | 1/164 | 28092 | AccessShareLock | t
test_mnist_packed | relation | | 4/122 | 28078 | AccessShareLock | t
model_arch_table_mnist | relation | | 4/122 | 28078 | AccessShareLock | t
model_arch_table_mnist | relation | | 1/164 | 28091 | AccessShareLock | t
mst_diag_table_hb_mnist | relation | | 1/164 | 28092 | AccessShareLock | t
mst_diag_table_hb_mnist | relation | | 1/164 | 28091 | AccessShareLock | t
mst_diag_table_hb_mnist | relation | | 4/122 | 28078 | AccessShareLock | t
mst_diag_table_hb_mnist_summary | relation | | 1/164 | 28092 | AccessShareLock | t
mst_diag_table_hb_mnist_summary | relation | | 1/164 | 28091 | AccessShareLock | t
mst_diag_table_hb_mnist_summary | relation | | 4/122 | 28078 | AccessShareLock | t
mnist_multi_model | relation | | 4/122 | 28078 | ExclusiveLock | t
mnist_multi_model | relation | | 4/122 | 28078 | RowExclusiveLock | t
mnist_multi_model | relation | | 4/122 | 28078 | AccessShareLock | t
mnist_multi_model | relation | | 1/164 | 28092 | RowExclusiveLock | t
mnist_multi_model | relation | | 1/164 | 28091 | AccessShareLock | t
mnist_multi_model | relation | | 1/164 | 28091 | RowExclusiveLock | t
mnist_multi_model | relation | | 1/164 | 28092 | AccessShareLock | t
mnist_multi_model_info | relation | | 1/164 | 28092 | AccessExclusiveLock | t
mnist_multi_model_info | relation | | 1/164 | 28092 | AccessShareLock | t
mnist_multi_model_info | relation | | 4/122 | 28078 | AccessShareLock | t
mnist_multi_model_info | relation | | 1/164 | 28091 | AccessExclusiveLock | t
mnist_multi_model_info | relation | | 1/164 | 28091 | AccessShareLock | t
mnist_multi_model_info | relation | | 4/122 | 28078 | AccessExclusiveLock | t
mnist_multi_model_summary | relation | | 4/122 | 28078 | AccessShareLock | t
mnist_multi_model_summary | relation | | 1/164 | 28091 | AccessShareLock | t
mnist_multi_model_summary | relation | | 1/164 | 28092 | AccessShareLock | t
```
I am not sure this is actually a problem, however, but wanted to note it here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] asfgit closed pull request #466: DL: Modify multi-fit warm
start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] asf-ci commented on issue #466: DL: Modify multi-fit warm
start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
asf-ci commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-568125818
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/1185/
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] khannaekta commented on a change in pull request #466: DL:
Modify multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [madlib] khannaekta commented on a change in pull request #466: DL:
Modify multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
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_r362952018
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_fit_multiple_model.py_in
##########
@@ -352,6 +353,22 @@ class FitMultipleModel():
metrics_type=metrics_type)
plpy.execute(info_table_insert_query)
+ if not mst['mst_key'] in warm_start_msts:
+ model = model_from_json(model_arch)
+ serialized_state = model_weights if model_weights else \
+ madlib_keras_serializer.serialize_nd_weights(model.get_weights())
Review comment:
I think we don't need to calculate the weights here for non warm_start mst_keys as we have already calculated it from the model when calling `get_initial_weights()`. So we can just use `serialized_weights` to insert into model_output_table.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 commented on issue #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-568138854
after quick initial look: might need to truncate info and summary tables
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 commented on issue #466: DL: Modify multi-fit
warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571779634
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] fmcquillan99 edited a comment on issue #466: DL: Modify
multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 edited a comment on issue #466: DL: Modify multi-fit warm start to accept non-matching mst&model tables
URL: https://github.com/apache/madlib/pull/466#issuecomment-571779179
Seems to pick up from where it left off when warm start is on, though these curves are somewhat flat so not super clear (see pics below).
```
SELECT validation_loss, validation_metrics FROM results_mnist where s=2 AND mst_key = 8;
```
results in
```
-[ RECORD 1 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0454168096184731}
validation_metrics | {0.986400008201599}
-[ RECORD 2 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0372964888811111,0.0447742976248264,0.0334407761693001}
validation_metrics | {0.98769998550415,0.986100018024445,0.989400029182434}
-[ RECORD 3 ]------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
validation_loss | {0.0316416546702385,0.0328738279640675,0.0300650335848331,0.0358279161155224,0.0323219522833824,0.0301002375781536,0.0322590619325638,0.0325789600610733,0.0320237688720226}
validation_metrics | {0.98989999294281,0.989600002765656,0.990899980068207,0.988699972629547,0.991400003433228,0.991999983787537,0.989700019359589,0.990400016307831,0.990899980068207}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [madlib] khannaekta commented on a change in pull request #466: DL:
Modify multi-fit warm start to accept non-matching mst&model tables
Posted by GitBox <gi...@apache.org>.
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_r362956766
##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_fit_multiple_model.py_in
##########
@@ -274,65 +274,66 @@ class FitMultipleModel():
plpy.execute(mst_insert_query)
def create_model_output_table(self):
- output_table_create_query = """
- CREATE TABLE {self.model_output_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_weights_col} BYTEA,
- {self.model_arch_col} JSON)
- """.format(self=self)
- info_table_create_query = """
- CREATE TABLE {self.model_info_table}
- ({self.mst_key_col} INTEGER PRIMARY KEY,
- {self.model_id_col} INTEGER,
- {self.compile_params_col} TEXT,
- {self.fit_params_col} TEXT,
- model_type TEXT,
- model_size DOUBLE PRECISION,
- metrics_elapsed_time DOUBLE PRECISION[],
- metrics_type TEXT[],
- training_metrics_final DOUBLE PRECISION,
- training_loss_final DOUBLE PRECISION,
- training_metrics DOUBLE PRECISION[],
- training_loss DOUBLE PRECISION[],
- validation_metrics_final DOUBLE PRECISION,
- validation_loss_final DOUBLE PRECISION,
- validation_metrics DOUBLE PRECISION[],
- validation_loss DOUBLE PRECISION[])
- """.format(self=self)
-
- plpy.execute(output_table_create_query)
- plpy.execute(info_table_create_query)
+ warm_start_msts = []
+ if self.warm_start:
+ warm_start_msts = plpy.execute(
+ """ SELECT array_agg({0}) AS a FROM {1}
+ """.format(self.mst_key_col, self.model_output_table))[0]['a']
+ plpy.execute("TRUNCATE TABLE {0}".format(self.model_info_table))
Review comment:
We shouldn't be Truncating the info table. As for the following scenario,
- mst_table has 4 mst_keys
- run `madlib_keras_fit_multiple_model()` -> output and info table has 4 mst_key values
- Delete 1 mst_key, run `madlib_keras_fit_multiple_model()` with warm_start = True
With this PR:
output table has 4 mst_key values and info table has only 3 mst_key
On master:
output table and info table has 4 mst_key
We should also update the test `madlib_keras_transfer_learning.sql_in:228` to Fail for the above scenario. Currently, it passes the assert query returns 0 rows.
----------------------------------------------------------------
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
With regards,
Apache Git Services