Skip to content

Preserve preprocessing outputs in Grain pipelines - #2959

Open
maitry63 wants to merge 6 commits into
keras-team:masterfrom
maitry63:fix-grain-preprocessing-outputs
Open

Preserve preprocessing outputs in Grain pipelines#2959
maitry63 wants to merge 6 commits into
keras-team:masterfrom
maitry63:fix-grain-preprocessing-outputs

Conversation

@maitry63

Copy link
Copy Markdown
Contributor

This PR fixes, when a preprocessing function is executed inside a Grain data pipeline, its eager outputs should be returned without passing through convert_preprocessing_outputs().

Currently, @preprocessing_function always converts the output after the preprocessing function returns. This can convert eager Python/NumPy outputs unnecessarily and can change ragged Python outputs such as list[list[...]].

This change reuses Keras core's existing in_grain_data_pipeline() detection and skips output conversion when running inside a Grain data pipeline.

Outside Grain pipelines, the existing convert_preprocessing_outputs() behavior is unchanged.

Fixes: #2945

Added regression tests covering:

  • Python ragged outputs remain list[list[...]] inside Grain pipelines.
  • Outputs continue to be converted normally outside Grain pipelines.

Checklist

  • I have added all the necessary unit tests for my change.
  • I have verified that my change does not break existing code and works with all backends (TensorFlow, JAX, and PyTorch).
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have followed the Keras Hub Model contribution guidelines in making these changes.
  • I have followed the Keras Hub API design guidelines in making these changes.
  • I have signed the Contributor License Agreement.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the preprocessing wrapper in tensor_utils.py to skip output conversion when running inside a Grain data pipeline, and adds corresponding unit tests. The review feedback suggests wrapping the import of in_grain_data_pipeline in a try-except block to prevent ImportError on older versions of Keras 3, ensuring backward compatibility.

import keras
import numpy as np
from keras import ops
from keras.src.utils.backend_utils import in_grain_data_pipeline

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Importing in_grain_data_pipeline directly from keras.src.utils.backend_utils can cause an ImportError on older versions of Keras 3 where this function is not yet defined. To maintain backward compatibility and prevent import failures, we should wrap this import in a try-except block and provide a fallback implementation that returns False.

Suggested change
from keras.src.utils.backend_utils import in_grain_data_pipeline
try:
from keras.src.utils.backend_utils import in_grain_data_pipeline
except ImportError:
def in_grain_data_pipeline():
return False
References
  1. Demand Robustness: Do not accept fragile code. If the proposed code is not robust enough or lacks proper error handling, explicitly tell the author why the current approach is brittle and what must be done to reinforce it. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the latest 3.15.1 Keras version in_grain_data_pipeline is available. I verified the direct import against the released latest package, so I kept the direct import.

Comment thread keras_hub/src/utils/tensor_utils.py Outdated
Comment on lines +68 to +70
def convert_outputs(x):
if in_grain_data_pipeline():
return x

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix never fires on the eager/python path - convert_preprocessing_outputs_python needs the same guard

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blanket skip regresses ragged/string tf outputs to tf.RaggedTensor- convert to NumPy/lists instead of skipping

@maitry63 maitry63 Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @divyashreepathihalli , thank you for the review.
Added the in_grain_data_pipeline() check to convert_preprocessing_outputs_python so the fix now correctly fires on eager and Python-based execution paths.

Removed the blanket return x inside Grain pipelines.

@@ -71,7 +78,7 @@ def wrapper(self, x, **kwargs):
x = convert_preprocessing_inputs(x)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input conversion still creates backend tensors in Grain workers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the in_grain_data_pipeline() check to the top of convert_preprocessing_inputs to ensure input conversion correctly bypasses backend tensor creation inside Grain workers.


test(self, ([1, 2, 3], ["foo", "bar"], "foo"))

def test_preprocessing_function_skips_output_conversion_in_grain(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mocked detection can't catch either bug- add a real grain.MapDataset integration test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the mocked patch test with a real grain.MapDataset integration test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PyGrain Migration] 1. Output NumPy (not backend tensors) from preprocessing inside Grain pipelines

2 participants