Skip to content

PR #4640: fix: refine scanned layers unroll logic for Gemma 3 - #4693

Open
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_955361604
Open

PR #4640: fix: refine scanned layers unroll logic for Gemma 3#4693
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_955361604

Conversation

@copybara-service

Copy link
Copy Markdown
Contributor

PR #4640: fix: refine scanned layers unroll logic for Gemma 3

Imported from GitHub PR #4640

Description

The unroll_gemma_scanned_weights workaround—specifically designed to unstack Gemma 3 scanned blocks due to Tunix's limitations—was previously formatting the unrolled keys incorrectly (using raw integers for the layer index). This PR fixes :

  1. the key generation logic to correctly format the unrolled layers as strings (e.g., changing the tuple injection from (0,) to ("layers_0",)), which aligns with the expected dictionary structure for downstream weight mapping.
  2. it refines the scan_length detection to safely check for the mlp.wi_0 architectural pattern.

Tests

We run train_rl.py for both scanned and unscanned layers. Both should return normal sentences

Scanned layers

Config: scan_layers=true and use_standalone_converter=True

  • [internal link]
  • [internal link]

Unscanned layers

Config : scan_layers=false

  • [internal link]
  • [internal link]

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.

Copybara import of the project:

--
8e1b8f3 by mesakhcienet mesakh.christian@cienet.com:

Merging this change closes #4640

FUTURE_COPYBARA_INTEGRATE_REVIEW=#4640 from AI-Hypercomputer:fix/train-rl-layers 8e1b8f3

Imported from GitHub PR #4640

# Description

   The `unroll_gemma_scanned_weights` workaround—specifically designed to unstack Gemma 3 scanned blocks due to Tunix's limitations—was previously formatting the unrolled keys incorrectly (using raw integers for the layer index). This PR fixes :
1. the key generation logic to correctly format the unrolled layers as strings (e.g., changing the tuple injection from `(0,)` to `("layers_0",)`), which aligns with the expected dictionary structure for downstream weight mapping.
2. it refines the `scan_length` detection to safely check for the `mlp.wi_0` architectural pattern.

# Tests

We run `train_rl.py` for both scanned and unscanned layers. Both should return normal sentences

## Scanned layers
Config: `scan_layers=true` and `use_standalone_converter=True`
- [internal link]
- [internal link]

## Unscanned layers
Config : `scan_layers=false`
- [internal link]
- [internal link]

# Checklist

Before submitting this PR, please make sure (put X in square brackets):
- [x] I have performed a self-review of my code. For an optional AI review, add the `gemini-review` label.
- [x] I have necessary comments in my code, particularly in hard-to-understand areas.
- [x] I have run end-to-end tests tests and provided workload links above if applicable.
- [x] 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](https://maxtext.readthedocs.io/en/latest/development.html#adding-new-documentation-files).

Copybara import of the project:

--
8e1b8f3 by mesakhcienet <mesakh.christian@cienet.com>:

Merging this change closes #4640

FUTURE_COPYBARA_INTEGRATE_REVIEW=#4640 from AI-Hypercomputer:fix/train-rl-layers 8e1b8f3
PiperOrigin-RevId: 955361604
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