Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix sok import error in ut #1232

Merged
merged 2 commits into from
Jan 7, 2024
Merged

Fix sok import error in ut #1232

merged 2 commits into from
Jan 7, 2024

Conversation

EmmaQiaoCh
Copy link
Contributor

SOK has refactored the code path and remove experiment. There are ut failure in tests/unit/tf/horovod/test_embedding.py due to below error:
ImportError: 'horovod' and 'sparse_operation_kit' are required to use SOKEmbedding.

@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 17, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@EmmaQiaoCh EmmaQiaoCh self-assigned this Dec 17, 2023
@EmmaQiaoCh EmmaQiaoCh added 23.12 enhancement New feature or request labels Dec 17, 2023
minseokl
minseokl requested changes Dec 17, 2023
View reviewed changes
Copy link

@minseokl minseokl left a comment

Choose a reason for hiding this comment

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

Please just do import sparse_operation_kit as sok

edknv
edknv approved these changes Jan 7, 2024
View reviewed changes
Copy link
Contributor

@edknv edknv left a comment

Choose a reason for hiding this comment

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

The failing tests in CI are unrelated to this PR and are fixed in separate PRs. The failure in horovod-cpu is fixed with #1235 and docs-build is fixed with #1236. gpu-multi / tensorflow is expected to fail since we no longer have access to multi-gpu instances.

@edknv edknv mentioned this pull request Jan 7, 2024
Merged
@edknv edknv merged commit 162f498 into main Jan 7, 2024
39 of 42 checks passed
@edknv edknv deleted the pull-request/1231 branch January 7, 2024 07:00
@edknv edknv mentioned this pull request Jan 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@minseokl minseokl minseokl approved these changes

@edknv edknv edknv approved these changes

Assignees

@EmmaQiaoCh EmmaQiaoCh

Labels
23.12 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EmmaQiaoCh @minseokl @edknv