Skip to content

Fix AttributeError on missing use_sft in grain data processing - #5137

Open
bvandermoon wants to merge 1 commit into
mainfrom
bvandermoon-fix-grain-multimodal-sft-eval
Open

Fix AttributeError on missing use_sft in grain data processing#5137
bvandermoon wants to merge 1 commit into
mainfrom
bvandermoon-fix-grain-multimodal-sft-eval

Conversation

@bvandermoon

@bvandermoon bvandermoon commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes an AttributeError: 'types.SimpleNamespace' object has no attribute 'use_sft' in make_grain_eval_iterator caused by a collision between PR #4754 and PR #4625.

  • In src/maxtext/input_pipeline/grain_data_processing.py, safely check use_sft, use_dpo, and use_multimodal with getattr(config, ..., False) in _get_pipeline_fn, make_grain_train_iterator, and make_grain_eval_iterator.
  • In tests/unit/mmap_data_processing_test.py, define use_sft=False and use_multimodal=False in GrainMmapNpyEvalConfigTest._config.

Tests

  • pytest tests/unit/mmap_data_processing_test.py -k "GrainMmapNpyEvalConfigTest" passes.
  • All 161 tests in tests/unit/mmap_data_processing_test.py pass.

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed, including adding new documentation pages to the relevant Table of Contents (toctree directive) as explained in our documentation.

Safely check for use_sft, use_dpo, and use_multimodal using getattr(config, ..., False) in grain data processing pipelines.
Also add use_sft=False and use_multimodal=False to GrainMmapNpyEvalConfigTest._config so that mock configs in unit tests provide these fields.

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

Copy link
Copy Markdown

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 input pipeline data processing code to use getattr with default values when accessing configuration attributes like use_sft, use_multimodal, use_dpo, train_image_column, and eval_image_column. This prevents potential AttributeError exceptions when these configuration options are not explicitly defined. Additionally, default values for use_sft and use_multimodal were added to the test configuration helper. There are no review comments, so I have no feedback to provide.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...rc/maxtext/input_pipeline/grain_data_processing.py 0.00% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

1 participant