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

Switches dependencies from txt file to toml file #68

Merged
merged 12 commits into from
Mar 27, 2024

Conversation

jbusche
Copy link
Collaborator

@jbusche jbusche commented Mar 2, 2024

Description of the change

This PR creates a pyproject.toml file and refers to the two requirements.txt and flashattn_requirements.txt files for dependencies.

  • It creates a pyproject.toml that points to the two requirements.txt and flashattn_requirements.txt files
  • Updates the README.md to reflect the new method to install
  • Updates the CONTRIBUTING.md to reflect the new method to install
  • Updates the build/Dockerfile to reflect the new method to install
  • Tightens up the requirements.txt to as small as possible
  • In the pyproject.toml, there's opportunity to specify more information about the project, like the name, description, authors, maintainers, public URLS, etc. I've added some people and urls into it, but it's open to discussion to whom else might need to be added or if more changes are needed.

Related issue number

Closes issue #56

How to verify the PR

I've been doing this:

On a Red Hat system (or a Mac system, skipping step 2 below)

  1. Download the branch
git clone https://github.com/jbusche/fms-hf-tuning -b jb-dependency-issue56
cd fms-hf-tuning/
  1. Install python39 if you don't already have it:
yum install https://dl.fedoraproject.org/pub/epel/epel-release-latest-8.noarch.rpm -y
yum install python39 -y
  1. Startup a python virtual environment:
python3.9 -m venv .env
source .env/bin/activate
  1. Install with this command:
pip install -e .
  1. You can test installing the optional FlashAttention dependencies with this command, though they'll complain you don't have CUDA if you don't have CUDA installed.
pip install -e ".[dev]"
pip install -e ".[flash-attn]"
  1. Then you can try running with this command:
    Note: you'll need a twitter_complaints.json file that I can send you.
export CUDA_VISIBLE_DEVICES=0
export WORLD_SIZE=1
export MODEL_PATH="bigscience/bloomz-560m"
export DATA_PATH="twitter_complaints.json"
export OUTPUT_PATH="/tmp/fms-hf-tuning/output"
python -m tuning.sft_trainer  \
--model_name_or_path $MODEL_PATH  \
--training_data_path $DATA_PATH  \
--output_dir $OUTPUT_PATH  \
--num_train_epochs 1  \
--per_device_train_batch_size 4  \
--per_device_eval_batch_size 4  \
--gradient_accumulation_steps 4  \
--evaluation_strategy "no"  \
--save_strategy "epoch"  \
--learning_rate 1e-5  \
--weight_decay 0.0  \
--lr_scheduler_type "cosine"  \
--logging_steps 1  \
--packing false  \
--include_tokens_per_second true \
--response_template "\n### Response:"  \
--dataset_text_field "output" \
--tokenizer_name_or_path "bigscience/tokenizer" \
--use_flash_attn false \
--torch_dtype "float32" \
--peft_method "pt"

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
  • Manually tested in the above steps

@jbusche
Copy link
Collaborator Author

jbusche commented Mar 5, 2024

Thank you for the help @lchu-ibm and @FilM for the help, I've switched back to the txt files and having the toml file pull up the dependencies from the txt files.

requirements.txt Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@jbusche
Copy link
Collaborator Author

jbusche commented Mar 11, 2024

@gkumbhat and @Ssukriti I think I've accounted for all the suggestions and changes... hoping that this is good to go.

requirements.txt Outdated Show resolved Hide resolved
@jbusche
Copy link
Collaborator Author

jbusche commented Mar 11, 2024

Does someone have the ability to test the flash-attn pip install -e .[flash-attn]? I don't have a GPU location to install CUDA and test with it.

@tedhtchang
Copy link
Collaborator

I think @z103cb should review this.

@jbusche
Copy link
Collaborator Author

jbusche commented Mar 25, 2024

Thanks @tedhtchang, yes we need this so we can start with creating the wheel. @gkumbhat and @Ssukriti can you review too?

requirements.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@gkumbhat gkumbhat left a comment

Choose a reason for hiding this comment

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

I noticed that the project.name defined here is fms-hf-tuning, but the package being installed is tuning, probably because of the name of the main folder being tuning. This makes the installation a bit confusing IMO.

Also I tried the pip install -e ".[flash-attn], but couldn't get flash-att get installed as separate dependency

@jbusche
Copy link
Collaborator Author

jbusche commented Mar 25, 2024

Oh my, well my rebase went crazy, was trying to fix the requirements file but now I'm getting all the last 3-4 weeks of changes, causing conflicts. Let me see if I can recover. Sorry

jbusche and others added 11 commits March 25, 2024 12:25
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Bumps [aim](https://github.com/aimhubio/aim) from 3.17.5 to 3.18.1.
- [Release notes](https://github.com/aimhubio/aim/releases)
- [Changelog](https://github.com/aimhubio/aim/blob/main/CHANGELOG.md)
- [Commits](aimhubio/aim@v3.17.5...v3.18.1)

---
updated-dependencies:
- dependency-name: aim
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
* Add Contributing file

Signed-off-by: James Busche <jbusche@us.ibm.com>

* add issue templates

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Adding Alex to CODEOWNERS

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Add ADR and squash info

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Update CONTRIBUTING.md

Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>

* NIT: Update CONTRIBUTING.md

Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>

---------

Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
Co-authored-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
* Add Contributing file

Signed-off-by: James Busche <jbusche@us.ibm.com>

* add issue templates

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Adding Alex to CODEOWNERS

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Add ADR and squash info

Signed-off-by: James Busche <jbusche@us.ibm.com>

* Update CONTRIBUTING.md

Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>

* NIT: Update CONTRIBUTING.md

Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>

---------

Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
Co-authored-by: Sukriti Sharma <Ssukriti@users.noreply.github.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Copy link

@z103cb z103cb 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 looking good to me.

@jbusche
Copy link
Collaborator Author

jbusche commented Mar 26, 2024

I'm still testing and making sure I understand/address the changes requested.

Signed-off-by: James Busche <jbusche@us.ibm.com>
@jbusche
Copy link
Collaborator Author

jbusche commented Mar 26, 2024

I've made the name change to fms-hf-tuning from just tuning, thank you @gkumbhat for the suggestion.

@jbusche jbusche requested a review from gkumbhat March 26, 2024 22:03
Copy link
Collaborator

@gkumbhat gkumbhat left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for contributing and making all the changes!

One suggestion for future iteration: We can look into getting rid of setup.py by defining py-modules or packages in tool.setuptools in pyproject.toml

@gkumbhat gkumbhat merged commit 4cc09cb into foundation-model-stack:main Mar 27, 2024
3 checks passed
@jbusche jbusche deleted the jb-dependency-issue56 branch April 2, 2024 20:44
@gkumbhat gkumbhat mentioned this pull request Apr 12, 2024
2 tasks
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.

None yet

5 participants