github-actions[bot] commented on code in PR #40667:
URL: https://github.com/apache/beam/pull/40667#discussion_r4318002055
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -2237,26 +2240,29 @@ def process(
side input is empty or the model has not been updated, the method
simply runs inference on the batch of data.
"""
- if not si_model_metadata:
+ if (not si_model_metadata or
+ isinstance(si_model_metadata, beam.pvalue.EmptySideInput)):
+ # No side input value: keep the loaded model, if any.
if (not self._model_metadata.is_valid_tag(self._cur_tag) or
self._model is None):
self.update_model(side_input_model_path=None)
return self._run_inference(batch, inference_args)
- if isinstance(si_model_metadata, beam.pvalue.EmptySideInput):
- self.update_model(side_input_model_path=None)
- elif isinstance(si_model_metadata, list) and hasattr(si_model_metadata[0],
- 'keys'):
+ if isinstance(si_model_metadata, list) and hasattr(si_model_metadata[0],
+ 'keys'):
# TODO(https://github.com/apache/beam/issues/27628): Update metrics here
- self.update_model(si_model_metadata)
+ needs_update = (
+ self._model is None or # No model loaded yet.
+ self._side_input_path != si_model_metadata or # Mapping changed.
+ not self._model_metadata.is_valid_tag(self._cur_tag)) # Invalidated.
+ if needs_update:
+ self.update_model(si_model_metadata)
+ self._side_input_path = list(si_model_metadata)
elif self._side_input_path != si_model_metadata.model_id:
Review Comment:
**🟡 Medium**: Unlike the empty side-input branch (line 2246) and the
`KeyModelPathMapping` branch (line 2257), this `ModelMetadata` branch only
checks `self._side_input_path != si_model_metadata.model_id`. If the currently
loaded model was marked invalid due to an inference timeout (`not
self._model_metadata.is_valid_tag(self._cur_tag)`), or `self._model is None`,
the model will not be reloaded until `si_model_metadata.model_id` changes.
Consider checking `self._model is None` and `not
self._model_metadata.is_valid_tag(self._cur_tag)` here as well for consistency.
```suggestion
elif (self._model is None or self._side_input_path !=
si_model_metadata.model_id or not
self._model_metadata.is_valid_tag(self._cur_tag)):
```
##########
CHANGES.md:
##########
@@ -94,7 +94,6 @@
## Deprecations
* Support for Python 3.10 was removed following its end-of-life
([#40250](https://github.com/apache/beam/issues/40250)).
Review Comment:
**🟢 Low**: Please keep the `* X behavior is deprecated...` and `* Fixed X
(Java/Python)...` placeholder lines in the unreleased section of `CHANGES.md`,
as they serve as section templates for other contributors until the release is
cut.
--
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]
