[GitHub] [madlib] khannaekta commented on a change in pull request #463: DL: Add asymmetric cluster check for predict

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

khannaekta commented on a change in pull request #463: DL: Add asymmetric cluster check for predict
URL: https://github.com/apache/madlib/pull/463#discussion_r356265527
 
 

 ##########
 File path: src/ports/postgres/modules/deep_learning/madlib_keras_predict.py_in
 ##########
 @@ -244,14 +254,14 @@ class PredictBYOM(BasePredict):
 
 def internal_keras_predict(independent_var, model_architecture, model_weights,
                            is_response, normalizing_const, current_seg_id, seg_ids,
-                           images_per_seg, gpus_per_host, segments_per_host,
+                           images_per_seg, use_gpus, gpus_per_host, segments_per_host,
                            **kwargs):
     SD = kwargs['SD']
     model_key = 'segment_model_predict'
     row_count_key = 'row_count'
     try:
-        device_name = get_device_name_and_set_cuda_env_predict(gpus_per_host,
-                                                       current_seg_id)
+        device_name = get_device_name_and_set_cuda_env(
+            use_gpus, gpus_per_host, current_seg_id)
 
 Review comment:
   As we are already validating the gpu configuration prior to calling this function(for fit/evaluate/predict), the function `get_device_name_and_set_cuda_env()` should only care about setting device name (GPU or CPU based on the value of `gpus_per_host`) and should not validate GPU configuration(based on `use_gpus`). 
   I think, it's ok to remove `use_gpus` from `get_device_name_and_set_cuda_env()`, unless I am missing some scenario.  

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