[Tests] Fix CI for deprecated attention block when used with device_map
#9442
Conversation
| CUBLAS_WORKSPACE_CONFIG: :16:8 | ||
| run: | | ||
| python -m pytest -n 1 --max-worker-restart=0 --dist=loadfile \ | ||
| # DeprecatedAttentionBlockTests::test_conversion_when_using_device_map fails |
There was a problem hiding this comment.
do you know why this test would fail with -n 1 --dist=loadfile option? also did you tested it anywhere that it would work without this option?
I tried both
pytest -n 1 --dist=loadfile tests/models/test_attention_processor.py::DeprecatedAttentionBlockTests::test_conversion_when_using_device_mapand
pytest tests/models/test_attention_processor.py::DeprecatedAttentionBlockTests::test_conversion_when_using_device_mapboth were passing on my machine; but I can see it failed on CI so just wondering
There was a problem hiding this comment.
Yeah same. I tested on DGX and audace and both passed. So, no idea. Didn’t dig deeper because the blocks are deprecated anyway.
There was a problem hiding this comment.
so the test would pass if we remove this line "-n 1 --dist=loadfile" ?
There was a problem hiding this comment.
ok! I'm ok with the change then
what do you think @DN6 ?
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
| CUBLAS_WORKSPACE_CONFIG: :16:8 | ||
| run: | | ||
| python -m pytest -n 1 --max-worker-restart=0 --dist=loadfile \ | ||
| # DeprecatedAttentionBlockTests::test_conversion_when_using_device_map fails |
There was a problem hiding this comment.
ok! I'm ok with the change then
what do you think @DN6 ?
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
|
cc @DN6 is this ok to merge? |
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
|
@DN6 okay to merge? |
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
|
@DN6 okay to merge? |
|
Sorry missed this. Yes can merge. Failing tests are unrelated. |
|
Oh wait this was already fixed with this? PR: #10359 |
|
You're right |
What does this PR do?
Ran a round of fast GPU tests (from
push_tests.yml). They are all passing except for the deprecated attention block.I think the change is okay because it doesn't introduce any performance regressions in the CI, either.
The failure: https://github.com/huggingface/diffusers/actions/runs/10734214122/job/29768965396#step:6:4275