Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix run evaluation to get base model path #273

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

anhuong
Copy link
Collaborator

@anhuong anhuong commented Jul 31, 2024

Description of the change

The base model name is now required when running TunedCausalLM.load() due to recent change that went in which is breaking the run_evaluation script. run_evaluation does not have a way of passing in the base model path and did not need to pass this in because AutoPeftModelForCausalLM() handled loading the peft model.

This PR fixes this by:

  • Adding flag where user can pass in base_model_name_or_path
  • If user does not pass in base_model_name_or_path, then this value is tried to be pulled from the checkpoint's adapter_config.json

The flag is not required since if a user is running with a fine tuned model, there will be no adapter config nor a base model needed.

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

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Copy link
Collaborator

@aluu317 aluu317 left a comment

Choose a reason for hiding this comment

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

This is good to know, and a need to merge before I use evaluation script for quality testing. Just one comment inline for the fine tuning base model case

adapter_config = json.load(config_file)
base_model_name_or_path = adapter_config.get("base_model_name_or_path")

tuned_model = TunedCausalLM.load(
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I'm reading this correctly, this code is missing the case that a user is running evaluation on a fine tuned model and there's no adapter_config.json file and they don't pass in any value for base_model_name_or_path. Then the model will fail to load, and an exception is raised, is that correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point, the problem is that fine tuned models don't have an adapter_config.json nor is a base model needed to load the model. So this may be another bug in the run_inference script for fine tuned models that needs to be fixed. Let me try it out with fine tuned model...

Copy link
Collaborator Author

@anhuong anhuong Aug 8, 2024

Choose a reason for hiding this comment

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

@aluu317 verified that both changes did not affect the fine tuning case. No change is needed for fine tuning because of this line in TunedCausalLM.load(). Since no adapter_config.json is found, this jumps below to the except block where the model is correctly loaded as a merged model. Thus am able to run_evaluation with fine tuned models.

@aluu317
Copy link
Collaborator

aluu317 commented Aug 6, 2024

Should probably update https://github.com/foundation-model-stack/fms-hf-tuning/blob/main/scripts/evaluation.md#running-evaluation and put a note about fine-tuning vs. other cases

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Copy link
Collaborator

@aluu317 aluu317 left a comment

Choose a reason for hiding this comment

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

LGTM!

@anhuong anhuong merged commit bb0caf9 into foundation-model-stack:main Aug 8, 2024
7 checks passed
anhuong added a commit that referenced this pull request Aug 14, 2024
* Set default value of target_modules to be None in LoraConfig

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Removal of transformers logger and addition of python logger

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* FMT and lint check: Removal of transformers logger and addition of python logger

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* fix: remove lm_head for granite with llama arch models (#258)

* initial code for deleting lm_head

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix logic for copying checkpoint

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix check that embed_tokens and lm_head weights are the same

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix warning assertion

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix lm_head check, remove test

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* small fixes from code review

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fmt

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

---------

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Co-authored-by: Anh-Uong <anh.uong@ibm.com>
Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* Add config_utils tests

Signed-off-by: Angel Luu <angel.luu@us.ibm.com>

* Fix fmt

Signed-off-by: Angel Luu <angel.luu@us.ibm.com>

* Separate tests out and use docstrings

Signed-off-by: Angel Luu <angel.luu@us.ibm.com>

* Update more field/value checks from HF defaults

Signed-off-by: Angel Luu <angel.luu@us.ibm.com>

* Fix: Addition of env var TRANSFORMERS_VERBOSITY check

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* FMT Fix: Addition of env var TRANSFORMERS_VERBOSITY check

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* Add test for tokenizer in lora config (should be ignored)

Signed-off-by: Angel Luu <angel.luu@us.ibm.com>

* Adding logging support to accelerate launch

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* FMT_FIX: Adding logging support to accelerate launch

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* bug: On save event added to callback (#256)

* feat: On save event added to callback

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Removed additional bracket

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Removed additional bracket

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Format issues resolved

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: rebase with upstream and add new line

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

---------

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>
Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
Co-authored-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* feat: All metric handling changes (#263)

* feat: All metric handling changes

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Format issues

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

---------

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* feat: Configuration to set logging level for trigger log (#241)

* feat: Added the triggered login in the operation

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Formatting issues

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Added default config

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Moved the variable to right scope

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Checked added to validate config log level

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* fix: Removed some unwanted log file

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

---------

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* limit peft deps until investigate (#274)

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* Data custom collator (#260)

* refactor code to preprocess datasets

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* fix formatting

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* allow input/output in validate args

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* format input/output JSON and mask

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* function to return suitable collator

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* add tests for SFT Trainer input/output format

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* remove unused functions

Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* add eos token to input/output format

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* fix tests

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* improve docstrings

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* keeping JSON keys constant

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* support for input/output format

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* formatting fixes

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* update rEADME formats

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* formatting README

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

---------

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>
Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>

* Revert "limit peft deps until investigate (#274)" (#275)

This reverts commit f57ff63.

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* feat: per process state metric (#239)

Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>

* Modify test to pass with target_modules: None

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Logging changes and unit tests added

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* feat: Add a dockerfile argument to enable aimstack (#261)

* Add a dockerfile argument at the end of final layer to enable aimstack.
Currenlty guarded by a dockerfile argument.

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

* Set the default value of ENABLE_AIM to false

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

---------

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

* Solved conflict with main

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* FMT:Fix Solved conflict with main

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* enabling tests for prompt tuning

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* feat: Support pretokenized (#272)

* feat: support pretokenized datasets

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* fix: rebase with upstream and review commits

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* fix: rebase with upstream and review commits

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* fix: rebase with upstream and review commits

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* consolidate collator code

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* add valuerrors for incorrect args

Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>

* feat: add unit tests for validate_data_args and format_dataset

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* feat: add unit tests for validate_data_args and format_dataset

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* feat: add unit tests for validate_data_args and format_dataset

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* feat: add unit tests for validate_data_args and format_dataset

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

---------

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>
Co-authored-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>
Co-authored-by: Alex Brooks <alex.brooks@ibm.com>

* Update packaging requirement from <24,>=23.2 to >=23.2,<25 (#212)

Updates the requirements on [packaging](https://github.com/pypa/packaging) to permit the latest version.
- [Release notes](https://github.com/pypa/packaging/releases)
- [Changelog](https://github.com/pypa/packaging/blob/main/CHANGELOG.rst)
- [Commits](pypa/packaging@23.2...24.1)

---
updated-dependencies:
- dependency-name: packaging
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Anh Uong <anh.uong@ibm.com>

* enabling tests for prompt tuning (#278)

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>
Co-authored-by: Anh Uong <anh.uong@ibm.com>

* fix: do not add special tokens for custom tokenizer (#279)

Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>

* PR changes for changing logger

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* fix: bug where the logger was not being used properly (#286)

Signed-off-by: Hari <harikrishmenon@gmail.com>

* Unit Tests changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* Add functionality to free disk space from Github Actions (#287)

* Add functionality to free disk space from Github Actions

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Add functionality to free disk space from Github Actions, relocate from build-and-publish.yaml to image.yaml

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Move freeing space step to before building image

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

---------

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* commented os.environ[LOG_LEVEL] in accelerate.py for testing

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* PR changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* FIX:FMT

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* PR Changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* PR Changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* Add unit test to verify target_modules defaults correctly (#281)

* Add unit test to verify target_modules defaults correctly

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Add sft_trainer.main test to ensure target modules properly default for LoRA when set to None from CLI

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* fmt

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Use model_args instead of importing, fix nits

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Add test to ensure target_modules defaults to None in job config

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* Add additional check, fix nits

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

---------

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>

* docs: Add documentation on experiment tracking. (#257)

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

* Ensure additional metadata to trackers don't throw error in happy case. (#290)

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

* PR Changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* fix multiple runid creation bug with accelerate. (#268)

Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>

* feat: logging control operation (#264)

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* Metrics file epoch indexing from 0

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* Revert last commit

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* fix run evaluation to get base model path (#273)

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* PR Changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* PR Changes

Signed-off-by: Abhishek <maurya.abhishek@ibm.com>

* feat: Added additional events such as on_step_begin, on_optimizer_step, on_substep_end (#293)

Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>

* Always update setuptools to latest (#288)

Signed-off-by: James Busche <jbusche@us.ibm.com>
Co-authored-by: Anh Uong <anh.uong@ibm.com>

* Rename all fixtures with correct .jsonl extension (#295)

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>
Co-authored-by: Anh Uong <anh.uong@ibm.com>

* feat: add save_model_dir flag where final checkpoint saved (#291)

* add save_model_dir flag for final checkpoint

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* remove output_dir logic, add save method

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* update accelerate_launch, remove save tokenizer

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix: put back creation of .complete file

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* fix failing tests and add new ones

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* tests: add sft_trainer test to train and save

- small refactor of tests

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* add docs on saving checkpoints and fix help msg

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* update example and note best checkpoint

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* changes based on PR review

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

* add logging to save, fix error out properly

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

---------

Signed-off-by: Anh-Uong <anh.uong@ibm.com>

---------

Signed-off-by: Will Johnson <mwjohnson728@gmail.com>
Signed-off-by: Abhishek <maurya.abhishek@ibm.com>
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Signed-off-by: Angel Luu <angel.luu@us.ibm.com>
Signed-off-by: Padmanabha V Seshadri <seshapad@in.ibm.com>
Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
Signed-off-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>
Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Signed-off-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Hari <harikrishmenon@gmail.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Co-authored-by: Abhishek <maurya.abhishek@ibm.com>
Co-authored-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
Co-authored-by: Anh-Uong <anh.uong@ibm.com>
Co-authored-by: Abhishek Maurya <124327945+Abhishek-TAMU@users.noreply.github.com>
Co-authored-by: Angel Luu <angel.luu@us.ibm.com>
Co-authored-by: Angel Luu <an317gel@gmail.com>
Co-authored-by: Padmanabha V Seshadri <seshapad@in.ibm.com>
Co-authored-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
Co-authored-by: Alex-Brooks <Alex.Brooks@ibm.com>
Co-authored-by: Hari <harikrishmenon@gmail.com>
Co-authored-by: Dushyant Behl <dushyantbehl@users.noreply.github.com>
Co-authored-by: Sukriti-Sharma4 <sukriti.sharma4@ibm.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: James Busche <jbusche@us.ibm.com>
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