ml-explore / mlx-examples

Examples in the MLX framework
MIT License
5.92k stars 838 forks source link

Fix NameError in lora.py when loading pretrained adapters #817

Closed nahakiole closed 3 months ago

nahakiole commented 3 months ago

The current released version of mlx_lm (0.14.3) from https://pypi.org/project/mlx-lm/#files contains a bug in the lora.py file. When attempting to load pretrained adapters using either the --resume_adapter_file command-line argument or the resume_adapter_file option in the lora_config.yaml file, a NameError is raised because the variable resume_adapter_file is not defined.

$ mlx_lm.lora --config lora_config.yaml
Loading configuration file lora_config.yaml
Loading pretrained model
Fetching 11 files: 100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 11/11 [00:00<00:00, 93966.08it/s]
Special tokens have been added in the vocabulary, make sure the associated word embeddings are fine-tuned or trained.
Loading datasets
Training
Traceback (most recent call last):
  File "mlx_lm.lora", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "mlx_lm/lora.py", line 274, in main
    run(types.SimpleNamespace(**args))
  File "mlx_lm/lora.py", line 247, in run
    train_model(args, model, tokenizer, train_set, valid_set, training_callback)
  File "mlx_lm/lora.py", line 173, in train_model
    print(f"Loading pretrained adapters from {resume_adapter_file}")
                                              ^^^^^^^^^^^^^^^^^^^
NameError: name 'resume_adapter_file' is not defined

This pull request fixes the issue by correcting the variable name in the print statement. Instead of using resume_adapter_file, it now uses args.resume_adapter_file, which is the correct variable holding the path to the pretrained adapter file.

Before:

if args.resume_adapter_file is not None:
    print(f"Loading pretrained adapters from {resume_adapter_file}")

After:

if args.resume_adapter_file is not None:
    print(f"Loading pretrained adapters from {args.resume_adapter_file}")

This fix ensures that the correct variable is used in the print statement, preventing the NameError from occurring, regardless of whether the resume_adapter_file is specified through the command-line argument or in the lora_config.yaml file.

nahakiole commented 3 months ago

Seems to have been changed here when refactoring code: https://github.com/ml-explore/mlx-examples/commit/c457a3f88bc7ccfd227bf8c7808cca51dba3e518