Skip to content

Fix QLoRA and Gemma4-small decoder issues for NNX#4503

Closed
hsuan-lun-chiang wants to merge 6 commits into
AI-Hypercomputer:mainfrom
CIeNET-International:fix/nnx_layers
Closed

Fix QLoRA and Gemma4-small decoder issues for NNX#4503
hsuan-lun-chiang wants to merge 6 commits into
AI-Hypercomputer:mainfrom
CIeNET-International:fix/nnx_layers

Conversation

@hsuan-lun-chiang

@hsuan-lun-chiang hsuan-lun-chiang commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes several issues with the NNX decoders introduced in PR #4464:

  1. Fixes sequential mode layer update in NNXDecoder where it was mutating a temporary list returned by __getattr__ instead of updating the individual layers_i attributes. This was causing newly initialized parameters (like LoRA parameters) to be discarded.
  2. Fixes Gemma4-small tests failing due to TPU OOM on dev VMs by reducing test size.
  • I have performed a self-review of my code.
  • I have run unit tests locally and they pass.
  • Verified tests pass on remote TPU VM.

TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032

xibinliu and others added 4 commits July 15, 2026 00:58
…oading

Fix the Errors:
1. DeepSeek Unscanned Checkpoint Mismatch:
   When restoring unscanned DeepSeek checkpoints, loading failed with:
   'ValueError: Checkpoint structure mismatch: 374 of 377 model parameter paths were not found... Example missing paths: decoder.dense_layer_0...'
   Cause: _init_sequential_deepseek registered layers as singular 'dense_layer' and 'moe_layer', producing PyTree paths like 'decoder.dense_layer_0'. However, Linen checkpoints and MaxText converter scripts format layer keys as plural ('decoder.dense_layers_0', 'decoder.moe_layers_0').

2. Generic Unscanned Model Checkpoint Mismatch (GPT-OSS, Llama4, etc.):
   When restoring unscanned generic models, loading failed with:
   'ValueError: Checkpoint structure mismatch: 456 of 459 model parameter paths were not found... Example missing paths: decoder.layers.0...'
   Cause: _init_sequential_generic used _create_and_register_layer which appended to self.layers initialized as nnx.List([]). In Flax NNX, tracking submodules in an nnx.List container forces state PyTree extraction to use list-index paths ('decoder.layers.0') instead of named attribute paths ('decoder.layers_0'), breaking compatibility with on-disk Linen checkpoints.

Summary of Fixes:
- Updated _init_sequential_deepseek to use plural prefixes ('dense_layers', 'moe_layers').
- Used _create_and_register_named_layer in sequential initialization (_init_sequential_deepseek, _init_sequential_generic) so submodules register exclusively under named string attributes ('layers_0', 'layers_1').
- Replaced nnx.List with a plain Python list for self.layers and updated the forward pass to retrieve named attributes via getattr(self, f'layers_{lyr}').
- Sequential Layer Naming & Flax 0.12.0 Compatibility (nnx_decoders.py):
   - Fixed DeepSeek sequential layer retrieval in _apply_sequential_layers to look up 'dense_layers_{i}' and 'moe_layers_{i}' attributes when scan_layers=False.
   - Removed self.layers.append(layer) in _init_gemma4_small_layers to prevent Flax 0.12.0 from raising a static container attribute ValueError on unannotated Python lists.
- MaxEngine KV-Cache Stacking & Sharding (maxengine.py):
   - Updated self.prefill_kv_cache_shardings, _maybe_stack_prefill_result_cache, and _maybe_unstack_prefill_result_cache to handle named per-layer keys ('layers_0', 'layers_1') in addition to stacked 'layers' keys when scan_layers=False. Unstacks KV cache matching decode_state['cache'] keys to prevent PyTree structure mismatches during engine insert.
- LoRA Module Regex Pattern Matching (lora_utils.py & lora_utils_test.py):
   - Updated _get_lora_module_path regex replacement to r'layers(?:_[0-9]+|/[0-9]+)?/' so LoRA target verification matches named attributes ('decoder/layers_0/...') as well as indexed ('decoder/layers/0/...') and scanned ('decoder/layers/...') paths.
   - Updated expected regex string assertions in test_get_lora_module_path.
- Increase model size in lora_utils_test.py to avoid IndivisibleError on FSDP.
- Monkeypatch qwix LoraProvider.dot_general to find parameter names before quantization.
- Refactor NNXDecoder to support dynamic graph init with scan_layers=False.
- Add assertions to verify quantization in tests.

TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032
TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032
TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032
@hsuan-lun-chiang
hsuan-lun-chiang marked this pull request as draft July 16, 2026 02:56
TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032
When scan_layers=False, NNXDecoder updates its layers in a loop.
It was checking `hasattr(self, "layers")` to decide whether to update `self.layers[lyr]` or use `setattr(self, f"layers_{lyr}", new_layer)`.
Because of the newly added `__getattr__` fallback for read-only access to `self.layers`, `hasattr(self, "layers")` was evaluating to True, causing the code to try to mutate the dynamically constructed list in-place. This mutation was lost as it only modified the temporary list returned by `__getattr__`, leaving the actual `layers_{lyr}` attributes unchanged and discarding newly initialized parameters (like LoRA parameters).

This fix checks `hasattr(self, f"layers_{lyr}")` first to prioritize updating the individual layer attributes directly.

TAG=agy
CONV=a499f437-b38f-44a6-99d3-11a38fb2e032
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.

2 participants