Skip to content

TST Accelerate mixed adapter batches tests - #3630

Open
BenjaminBossan wants to merge 1 commit into
huggingface:mainfrom
BenjaminBossan:tst-accelerate-generate-mixed-adapter-batches-test
Open

TST Accelerate mixed adapter batches tests#3630
BenjaminBossan wants to merge 1 commit into
huggingface:mainfrom
BenjaminBossan:tst-accelerate-generate-mixed-adapter-batches-test

Conversation

@BenjaminBossan

Copy link
Copy Markdown
Member

The mixed adapter batches tests are relatively slow on CI, especially with MoE models. They constitute 4 out of the 5 slowest tests, with ~100 sec total:

27.26s call     tests/test_target_parameters.py::TestDecoderModelsTargetParameters::test_generate_with_mixed_adapter_batches[trl-internal-testing/tiny-GptOssForCausalLM-LoraConfig-config_kwargs7]
26.66s call     tests/test_poly.py::TestPoly::test_poly
26.38s call     tests/test_target_parameters.py::TestDecoderModelsTargetParameters::test_generate_with_mixed_adapter_batches[trl-internal-testing/tiny-GptOssForCausalLM-LoraConfig-config_kwargs4]
26.14s call     tests/test_target_parameters.py::TestDecoderModelsTargetParameters::test_generate_with_mixed_adapter_batches[trl-internal-testing/tiny-GptOssForCausalLM-LoraConfig-config_kwargs6]
25.44s call     tests/test_target_parameters.py::TestDecoderModelsTargetParameters::test_generate_with_mixed_adapter_batches[trl-internal-testing/tiny-GptOssForCausalLM-LoraConfig-config_kwargs5]

https://github.com/huggingface/peft/actions/runs/33380424434/job/99451723715#step:8:1512

The tests are a bit wasteful right now for MoE LoRA because target_parameters does not support this feature, so we check that trying to use it raises an appropriate error. The problem is that before checking that, we first run 3 expensive generate calls, which are not required to check the error.

This PR reorders the generate calls to call with mixed adapter batches first, triggering the error immediately. This is fine, as the order does not affect the results.

In my local testing on CPU, the total test time for this test went down from >90 sec to 30 sec, of which 20 sec are for test discovery.

To help a little more with speeding things up, I also decreased the max length and number of beams.

The test_mixed_adapter_batches test was updated too, mainly for consistency.

The mixed adapter batches tests are relatively slow on CI, especially
with MoE models. They constititute 4 out of the 5 slowest tests, with
~100 sec total:

https://github.com/huggingface/peft/actions/runs/33380424434/job/99451723715

The tests are a bit wasteful right now for MoE LoRA because
target_parameters does not support this feature, so we check that
trying to use it raises an appropriate error. The problem is that
before checking that, we first run 3 expensive generate calls, which
are not required to check the error.

This PR reorders the generate calls to call with mixed adapter batches
first, triggering the error immediately. This is fine, as the order
does not affect the results.

In my local testing on CPU, the total test time for this test went
down from >90 sec to 30 sec, of which 20 sec are for test discovery.

To help a little more with speeding things up, I also decreased the
max length and number of beams.

The test_mixed_adapter_batches test was updated too, mainly for
consistency.
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@BenjaminBossan

Copy link
Copy Markdown
Member Author

Update: With the CI finished, we can verify that the test_generate_with_mixed_adapter_batches tests are no longer among the 10 slowest tests:

https://github.com/huggingface/peft/actions/runs/33393694722/job/99493615262?pr=3630#step:8:1512

We can't see how long exactly they took, but it means that they went from > 25 sec to < 9 sec, saving us at least one minute on the CI.

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