- Notifications
You must be signed in to change notification settings - Fork6.1k
[ci] enable hotswapping tests on our nightly CI.#11826
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| env: | ||
| HF_TOKEN: ${{ secrets.DIFFUSERS_HF_HUB_READ_TOKEN }} | ||
| RUN_COMPILE: yes | ||
| run: | | ||
| python -m pytest -n 1 --max-worker-restart=0 --dist=loadfile -s -v -k "compile" --make-reports=tests_torch_compile_cuda tests/ | ||
| python -m pytest -n 1 --max-worker-restart=0 --dist=loadfile -s -v -k "hotswap" --make-reports=tests_torch_hotswap_cuda tests/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this would only run tests with the name hotswap in them and not all compile tests.
I think theis_torch_compile decorator andRUN_COMPILE env variable can only used to skip/include compile tests in the larger test suite. They can't be used to isolate compile tests only.
We should makeis_torch_compile a marker similar to#11786. And then add it to hotswap tests. That way you have more control over how compile tests run (you can include them as part of larger test suite, isolate them or skip them)
| run: cat reports/tests_torch_compile_cuda_failures_short.txt | ||
| run: | | ||
| cat reports/tests_torch_compile_cuda_failures_short.txt | ||
| cat reports/tests_torch_hotswap_cuda_failures_short.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can just group hotswap and compile together I think.
We don't run slow tests on merge to main. It takes to long to run them on every merge (> 1 hr) If we want to run compile tests on merge, we can just make fast test for them? |
And we can remove push_tests_fast. It's duplicate work now. Think I forgot to remove it. |
Discussed it with@DN6 offline. Will change the scope of the PR a bit to maintain consistency in terms of the types of tests being run during each stage (merge, nightly, etc.). |
Will club#11773 with it too. |
What does this PR do?
LoRA hotswapping with compilation is a crucial feature which deserves (at least nightly) testing. This PR enables that.
This PR also sets the
RUN_SLOWflag inpush_tests.ymlfor the compilation test job as without it, those tests won't run. Should we enable hotswapping tests in that workflow, too? I am okay if not.Additionally, a few questions:
push_tests_fast.ymlandpush_tests.yml. I am assuming the tests underpush_tests_fast.ymlwould have already run before the changes made their way tomain. So, I wonder if we should removepush_tests_fast.yml?push_tests? We should most definitely look into how we segregate slow and nightly our tests for this.I believe this deserves attention as it would help us reduce our CI time.