Skip to content

Conversation

@YashasviChaurasia
Copy link
Contributor

@YashasviChaurasia YashasviChaurasia commented Oct 8, 2025

Description of the change

This PR updates the codebase to switch from the external alora package to the native Activated LoRA (aLoRA) support that is now part of the Hugging Face peft library.
Key changes:

  • Removing legacy alora imports and wrappers.
  • Updating config_utils.py to handle aLoRA fields via peft.LoraConfig.
  • Adjusting tests to use alora_invocation_tokens for validation and inference.
  • Simplifying run_inference.py by removing custom aLoRA handling (now handled natively by PEFT).

Related issue number

How to verify the PR

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

@github-actions
Copy link

github-actions bot commented Oct 8, 2025

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

@github-actions github-actions bot added the feat label Oct 8, 2025
@YashasviChaurasia YashasviChaurasia marked this pull request as ready for review October 9, 2025 16:42
Copy link
Collaborator

@dushyantbehl dushyantbehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes.

@YashasviChaurasia
Copy link
Contributor Author

YashasviChaurasia commented Oct 10, 2025

@dushyantbehl Currently moving the --alora_invocation_string to LoraConfig is causing issue for the following reasons:

if peft_config is not None and hasattr(peft_config, "alora_invocation_string"):
    inv_str = getattr(peft_config, "alora_invocation_string")
    if not inv_str:
        raise ValueError(
            "`--alora_invocation_string` is required when using --peft_method alora."
        )
    ...

As now we use LoraConfig for both alora and lora runs setting up alora_invocation_string as a dataclass attribute in LoraConfig triggers the above code snippet causing it to fail as it is set to None by default if not provided by the user..
It seems simpler to keep it outside Dataclass and use it via additional as to avoid conflicts with Lora testcases.

Copy link
Collaborator

@ashokponkumar ashokponkumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work @YashasviChaurasia !

Copy link
Collaborator

@dushyantbehl dushyantbehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dushyantbehl dushyantbehl merged commit 5187516 into foundation-model-stack:main Oct 10, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants