Skip to content

[peft] revert: Validate PEFT target modules (#1747)#3583

Merged
ko3n1g merged 1 commit into
mainfrom
chcui/revert-pr-1747-peft-target-validation
Apr 29, 2026
Merged

[peft] revert: Validate PEFT target modules (#1747)#3583
ko3n1g merged 1 commit into
mainfrom
chcui/revert-pr-1747-peft-target-validation

Conversation

@cuichenx

Copy link
Copy Markdown
Contributor

Summary

Reverts #1747 ("feat: Validate PEFT target modules", merge commit 1396c27a67191fb4ddf5e9b97dbaebb12ee82378).

The newly added validation in module_matcher.py / canonical_lora.py rejects target-module configurations that real finetune recipes rely on, breaking finetune tests on main. Reverting unblocks finetune; the validation can be re-introduced in a follow-up that accommodates the existing recipe target lists.

Files reverted

  • src/megatron/bridge/peft/base.py
  • src/megatron/bridge/peft/canonical_lora.py
  • src/megatron/bridge/peft/dora.py
  • src/megatron/bridge/peft/module_matcher.py
  • tests/unit_tests/peft/test_canonical_lora.py
  • tests/unit_tests/peft/test_lora.py

Test plan

cc @yaoyu-33

This reverts commit 1396c27.

The validation broke finetune tests; reverting until a follow-up fix lands.

Signed-off-by: Chen Cui <chcui@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Apr 29, 2026

Copy link
Copy Markdown

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.

self._validate_target_matches()
self._reset_target_match_state()

self.freeze_model(model, training=training)

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.

LGTM — clean revert of #1747. The validation removal, canonical_lora.py else-branch restoration, and test adjustments are all consistent.

@ko3n1g ko3n1g 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.

ty!

@cuichenx

Copy link
Copy Markdown
Contributor Author

/ok to test e9cfe6f

@cuichenx cuichenx added the ready-to-merge PR is approved, current, and only waiting for CI to pass before merge label Apr 29, 2026
@ko3n1g ko3n1g enabled auto-merge (squash) April 29, 2026 21:13
@ko3n1g ko3n1g merged commit 9adb642 into main Apr 29, 2026
86 checks passed
@ko3n1g ko3n1g deleted the chcui/revert-pr-1747-peft-target-validation branch April 29, 2026 21:55
vasunvidia pushed a commit to vasunvidia/Megatron-Bridge that referenced this pull request Jun 10, 2026
…A-NeMo#3583)

Signed-off-by: Chen Cui <chcui@nvidia.com>
Signed-off-by: Vasudevan Rengasamy <vrengasamy@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge PR is approved, current, and only waiting for CI to pass before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants