damccorm commented on code in PR #29223:
URL: https://github.com/apache/beam/pull/29223#discussion_r1378202390
##########
sdks/python/apache_beam/ml/inference/huggingface_inference.py:
##########
@@ -638,6 +647,12 @@ def __init__(
if max_batch_size is not None:
self._batching_kwargs['max_batch_size'] = max_batch_size
self._large_model = large_model
+ if 'device' not in self._load_pipeline_args:
+ self._load_pipeline_args['device'] = self._device
+ else:
+ _LOGGER.warning(
+ '`device` specified in `load_pipeline_args`. `device` '
+ 'parameter for HuggingFacePipelineModelHandler will be ignored.')
Review Comment:
That also allows us to warn if the user specifies `GPU` but there's no gpu
available
##########
sdks/python/apache_beam/ml/inference/huggingface_inference.py:
##########
@@ -638,6 +647,12 @@ def __init__(
if max_batch_size is not None:
self._batching_kwargs['max_batch_size'] = max_batch_size
self._large_model = large_model
+ if 'device' not in self._load_pipeline_args:
+ self._load_pipeline_args['device'] = self._device
+ else:
+ _LOGGER.warning(
+ '`device` specified in `load_pipeline_args`. `device` '
+ 'parameter for HuggingFacePipelineModelHandler will be ignored.')
Review Comment:
I think we should only warn if a user explicitly passes in both device as an
arg and defines a device in `self._load_pipeline_args`. Otherwise this is
warning at a totally valid configuration.
As a result, I think we should default to `device: str = None` above, and
assume `None` means `GPU` (and doc it accordingly)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
