Skip to content

fix: biencoder PEFT adapter key remapping for merge_lora#1478

Open
adil-a wants to merge 3 commits intor0.3.0from
fix/biencoder-peft-key-remapping-r030
Open

fix: biencoder PEFT adapter key remapping for merge_lora#1478
adil-a wants to merge 3 commits intor0.3.0from
fix/biencoder-peft-key-remapping-r030

Conversation

@adil-a
Copy link
Collaborator

@adil-a adil-a commented Mar 7, 2026

Summary

  • Fix PEFT adapter key remapping in BiencoderStateDictAdapter so that merge_lora.py works against the standalone base model (LlamaBidirectionalModel)
  • The biencoder wraps the base model as lm_q, so adapter weights/target modules were saved with incorrect prefixes (lm_q.model. instead of lm_q. → bare)
  • LlamaBidirectionalModel extends LlamaModel (not LlamaForCausalLM), so its modules are layers.* not model.layers.*

Changes

  • state_dict_adapter.py: Fix PEFT key path in to_hf (base_model.model.lm_q.Xbase_model.model.X, was incorrectly producing base_model.model.model.X). Fix from_hf to handle the corrected format.
  • addons.py: Strip lm_q. prefix from target modules in _extract_target_modules for biencoder, so adapter_config.json has module names matching the standalone HF base model.

Test plan

  • Unit tests for PEFT key roundtrip (to_hffrom_hf)
  • Unit test for target module extraction with biencoder
  • All existing test_addons.py and test_state_dict_adapter.py tests pass (31 total)
  • End-to-end: run biencoder training → merge_lora.py against nvidia/llama-nemotron-embed-1b-v2

🤖 Generated with Claude Code

The biencoder wraps the base model as `lm_q`, so PEFT adapter weights
and target modules were saved with `lm_q.` prefix. However, the
standalone base model (LlamaBidirectionalModel extends LlamaModel) uses
bare module names like `layers.0.self_attn.q_proj` — no `model.` prefix.

This caused merge_lora.py to fail because PEFT could not match the
target modules or weight keys against the base model.

Changes:
- state_dict_adapter: fix PEFT key path in to_hf to strip `lm_q.`
  without adding `model.` (base_model.model.lm_q.X → base_model.model.X)
- state_dict_adapter: fix from_hf to handle new PEFT key format
  (base_model.model.X → base_model.model.lm_q.X)
- addons: strip `lm_q.` prefix from target modules for biencoder so
  adapter_config.json is compatible with the standalone HF base model

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@adil-a adil-a requested review from a team, HuiyingLi, akoumpa and hemildesai as code owners March 7, 2026 00:31
@copy-pr-bot
Copy link

copy-pr-bot bot commented Mar 7, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@akoumpa akoumpa added the r0.3.0 Add for cherry-pick into release branch r0.3.0 label Mar 7, 2026
@adil-a adil-a removed the r0.3.0 Add for cherry-pick into release branch r0.3.0 label Mar 7, 2026
@adil-a
Copy link
Collaborator Author

adil-a commented Mar 7, 2026

/ok to test b6c94c1

Signed-off-by: adil-a <adil.asif2000@hotmail.com>
@adil-a
Copy link
Collaborator Author

adil-a commented Mar 7, 2026

/ok to test 019acfc

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