TheNeuralBit commented on code in PR #17470:
URL: https://github.com/apache/beam/pull/17470#discussion_r878324657


##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -96,7 +107,9 @@ def expand(self, pcoll: beam.PCollection) -> 
beam.PCollection:
         pcoll
         # TODO(BEAM-14044): Hook into the batching DoFn APIs.
         | beam.BatchElements()
-        | beam.ParDo(_RunInferenceDoFn(self._model_loader, self._clock)))
+        | beam.ParDo(
+            _RunInferenceDoFn(
+                self._model_loader, self._prediction_params, self._clock)))

Review Comment:
   > But after a long discussion we settled on making these a side input.
   
   This is not a side input, this is a constant parameter. A side input comes 
from a PCollection wrapped in a PCollectionView and is passed to the ParDo: 
https://beam.apache.org/documentation/programming-guide/#side-inputs, and Beam 
handles passing actual data to the DoFn's process method.
   
   That's what I'm trying to suggest, making it a side input would justify a 
change to base RunInference, because it means you'd need to change the `ParDo` 
creation.
   
   



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

Reply via email to