tannnmayy opened a new pull request, #40929:
URL: https://github.com/apache/beam/pull/40929
[RunInference] Raise when a model handler returns fewer predictions than
inputs
**Body:**
Fixes #40928
`utils._convert_to_result` pairs each batch of inputs with the model's
predictions using `zip()`, which stops at the shorter of the two. When a
handler produced fewer predictions than inputs, the leftover inputs got no
result and no error, and `num_inferences` still counted them. That's how #36587
and #40913 went unnoticed, and it still happens with built-in handlers, for
example with PyTorch or Keras models that return a tuple of outputs, and with
Hugging Face causal LMs that return `past_key_values` (details in the issue).
This PR:
- makes `_convert_to_result` raise a `ValueError` when there are fewer
predictions than inputs. The message gives the counts, names any dict output
that doesn't have one entry per input, and suggests a fix (an inference
function that returns one prediction per input, or
`inference_args={'use_cache': False}` for Hugging Face models);
- leaves extra predictions ignored, as before, so pipelines in which every
input already gets a result keep working (for example, tuple outputs with batch
size 1);
- fixes three existing tests that only passed because of the truncation. In
`sklearn_inference_test.py`, `test_custom_inference_fn` used an inference
function that returned one prediction for three inputs, and its `zip()`-based
check compared only the first result. In `tensorflow_inference_test.py`, the
two env-var tests mocked the model, so their pipelines produced no results at
all. The fakes now return one prediction per input, and the scikit-learn test
also checks the number of results;
- adds unit tests for the helper, a RunInference test that checks the
affected inputs reach the failure output of `with_exception_handling()`, and a
CHANGES.md entry.
Testing:
- 9 new tests in `utils_test.py`. The 4 that check the new error fail
without this change; the other 5 cover behavior that doesn't change.
- The whole `apache_beam/ml` test suite with torch, TensorFlow,
transformers, ONNX, scikit-learn and XGBoost installed (Python 3.12): 609
passed, against 600 on master. The only differences are the 9 new tests. The
tests that can't run locally (they need GCP credentials or Docker) fail the
same way on both.
- The real-model cases from the issue: every case that dropped inputs now
raises, and the single-output controls still return one result per input.
- ruff, yapf and pyrefly report no issues.
The issue also asks whether RunInference should check the count itself, to
cover custom handlers that pair results with their own `zip()`. I left that out
to keep this change to the shared helper. If you'd prefer a warning instead of
an error, or the check in RunInference itself, I'm happy to change it.
R: @jrmccluskey
--
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]
