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

Workflow for GPU Runner #694

Merged
merged 8 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/unit_tests_gpu.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: Unit tests GPU

on:
push:
branches: [main]
pull_request:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We might not want the PR trigger here

Copy link
Collaborator

Choose a reason for hiding this comment

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

@riedgar-ms Does branches: [main] mean it'll only run when merged to main? brainstorming out loud, is there a way to manually trigger on PR instead of automatically? Another option is to have it run on a "staging" branch or something of the sort which we can merge to before merging with main. That way we can have a maintainer vet that the code isn't malicious, but still verify that it passes GPU tests before we put on main. Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

push ...main is (AIUI) the trigger which will run on the actual merge. The workflow_dispatch trigger is supposed to add a 'Run Workflow' button on the Actions page. Having said that, for the plain 'Unit Test' build, the list of available branches I'm seeing are only those for this repo, not forks.

We could have an extra 'staging' branch, but that would be an extra thing to manage. And if the workflow were being automatically run there, there would still be the potential problem of people sneaking in Bitcoin miners.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whenever there's a new contributor, I believe that the builds won't be run until someone listed on the guidance repo clicks on a button, so there is that, @Harsha-Nori

branches: [main]
workflow_dispatch:

jobs:
unit-tests:

strategy:
matrix:
os: [gpu-runner]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
model: ["gpt2gpu", "phi2gpu"]

runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Show GPUs
run: |
nvidia-smi
- name: Install dependencies
shell: bash
run: |
python -m pip install --upgrade pip
pip install pytest
pip install -e .[test]
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
- name: Install accelerate
run: |
pip install accelerate
- name: Check GPU available
run: |
python -c "import torch; assert torch.cuda.is_available()"
- name: Run tests (except server)
run: |
pytest --cov=guidance --cov-report=xml --cov-report=term-missing --selected_model ${{ matrix.model }} -m "not server" ./tests/
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
5 changes: 5 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
"phi2cpu": dict(
name="transformers:microsoft/phi-2", kwargs={"trust_remote_code": True}
),
"gpt2gpu": dict(name="transformers:gpt2", kwargs={"device_map": "cuda:0"}),
"phi2gpu": dict(
name="transformers:microsoft/phi-2",
kwargs={"trust_remote_code": True, "device_map": "cuda:0"},
),
}


Expand Down
4 changes: 2 additions & 2 deletions tests/library/test_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ def test_stop_char():
lm += "Count to 10: 1, 2, 3, 4, 5, 6, 7, " + gen('text', stop=",")
assert lm["text"] == "8"

def test_stop_list_side_effect():
def test_stop_list_side_effect(selected_model):
riedgar-ms marked this conversation as resolved.
Show resolved Hide resolved
'''Tests a bug where a stop list has an item appended to it in place instead of being updated non-destructively. The bug only occurs whe regex is also None'''
stop_list = ['\nStep', '\n\n', '\nAnswer'];
stop_list_length = len(stop_list);
lm = get_model("transformers:gpt2")
lm = selected_model
lm + '''Question: Josh decides to try flipping a house.  He buys a house for $80,000 and then puts in $50,000 in repairs.  This increased the value of the house by 150%.  How much profit did he make?
Let's think step by step, and then write the answer:
Step 1''' + gen('steps', list_append=True, stop=['\nStep', '\n\n', '\nAnswer'], temperature=0.7, max_tokens=20) + '\n'
Expand Down
Loading