Skip to content

Conversation

@dvrogozh
Copy link
Contributor

@dvrogozh
Copy link
Contributor Author

This seems sporadic ci failure unrelated to PR:

FAILED tests/utils/test_modeling_utils.py::ModelUtilsTest::test_from_pretrained_low_cpu_mem_usage_equal - AssertionError: 424.6015625 != 426.8125 within 2 delta (2.2109375 difference) : using `low_cpu_mem_usage` should incur the same memory usage in both cases.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Hey! Thanks, these functions are completely deprecated and the recommended API is to use accelerate's device_map = "auto" instead!

@dvrogozh
Copy link
Contributor Author

Hey! Thanks, these functions are completely deprecated and the recommended API is to use accelerate's device_map = "auto" instead!

Thank you for you feedback. I saw deprecation warnings and that's why I implemented this only for one model :). @ArthurZucker : I am trying to make ci passing clean for XPU backend. I totally understand if maintainers want to avoid changing deprecated stuff which will be dropped at some point. However I would like to raise a question on what can be done for the sake of clean ci? If decision is (and your reply hints it is) to not enable model_parallel for other non-CUDA backends, then associated tests should probably be marked as CUDA specific and excluded for non-CUDA. If that's the direction - I will submit another PR to mark these tests as CUDA specific.

@SunMarc
Copy link
Member

SunMarc commented Dec 13, 2024

Let's mark these tests as cuda specific. Thanks for your contribution!

@dvrogozh
Copy link
Contributor Author

Let's mark these tests as cuda specific. Thanks for your contribution!

Will do. Thank you for guidance!

@dvrogozh
Copy link
Contributor Author

I've submitted #35269 to mark tests with @require_torch_gpu.

@dvrogozh dvrogozh closed this Dec 13, 2024
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.

3 participants