Add trainer integration test for llava to ensure accelerate autocasting works correctly #30489
Conversation
| @@ -0,0 +1,120 @@ | |||
| import unittest | |||
There was a problem hiding this comment.
This file should be the only new change. That will be clearer once the other is merged. Usually I would set it up to merge this one into the first but this feels a bit difference since the changes are on a fork.
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
from
b723f1f
to
908ff93
Compare
|
|
||
| output = model(**inputs) | ||
| expected_slice = torch.tensor( | ||
| [[-3.5664, -3.5625, -0.4309], [-5.8242, -5.6914, -1.3242], [-5.4805, -5.9375, 1.1465]], |
There was a problem hiding this comment.
I saw this pattern in the llava next tests. These came from the use of this model before training. Not quite sure if this is correct so please let me know if there is something else we want to test. Perhaps instead we want the trained model before applying the downcasting change?
There was a problem hiding this comment.
Also note these do not yet pass allclose. I wanted to go ahead and open this PR to generate discussion around what the right thing to test is.
| "llava-hf/bakLlava-v1-hf", quantization_config=bits_and_bytes_config | ||
| ) | ||
| adapter_name = "lora_default" | ||
| peft_config = LoraConfig( |
There was a problem hiding this comment.
I am somewhat unclear on where we test the slow tests but I assume there is some limit on memory so I tried to give a reasonable LORA for this test. If you think there is a simpler or more idiomatic way to do this test please let me know.
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
from
908ff93
to
1cd13e0
Compare
| @slow | ||
| @require_bitsandbytes | ||
| def test_model_trainer_integration_test(self): | ||
| def image_prompt_generator(): |
There was a problem hiding this comment.
Not entirely sure this is the simplest or most idiomatic way to create this test dataset so please let me know if there is a better way.
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
4 times, most recently
from
7ce15e3
to
9440dd6
Compare
|
cc @muellerzr for first review |
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
from
9440dd6
to
e9e3feb
Compare
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
from
e9e3feb
to
bc31529
Compare
|
Updated this now that the previous PR is merged! I am very concerned about OOMs being an issue here however. I think there's some open questions around:
|
frasermince
force-pushed
the
frasermince/trainer-integration-test
branch
from
8f35687
to
3c7e7e1
Compare
What does this PR do?
This PR adds a new integration test to ensure the accelerate autocasting is working correctly. This came out of a discussion found here and that PR should probably be merged first (or this one merged into that one).
Fixes # (issue)
Before submitting
Pull Request section?
to it if that's the case.
Not an issue but another PR that should probably be merged first: Fix llava half precision and autocast issues #29721 (comment)
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.