[GitHub] [madlib] njayaram2 commented on issue #429: DL: Split madlib_keras dev-check file

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

njayaram2 commented on issue #429: DL: Split madlib_keras dev-check file
URL: https://github.com/apache/madlib/pull/429#issuecomment-519222467
 
 
   The changes LGTM. I had one concern, it's more of a trade-off though.
   I think the main intent of this PR is to make the tests manageable, and this PR surely does that well. But the total time to run the same tests now increases significantly:
   Time to run all the tests together in master:
   ```
   TEST CASE RESULT|Module: deep_learning|madlib_keras.sql_in|PASS|Time: 35243 milliseconds
   ```
   Time to run the same tests now split across 4 files in this PR branch:
   ```
   TEST CASE RESULT|Module: deep_learning|madlib_keras_evaluate.sql_in|PASS|Time: 6623 milliseconds
   TEST CASE RESULT|Module: deep_learning|madlib_keras_fit.sql_in|PASS|Time: 22986 milliseconds
   TEST CASE RESULT|Module: deep_learning|madlib_keras_predict.sql_in|PASS|Time: 17447 milliseconds
   TEST CASE RESULT|Module: deep_learning|madlib_keras_transfer_learning.sql_in|PASS|Time: 8327 milliseconds
   ```
   It is ~55 seconds now against ~35 seconds on master. It might be ok, and we should probably do this only for modules that have large dev-check files (such as deep learning).

----------------------------------------------------------------
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