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: [email protected] With regards, Apache Git Services
