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

[XLA:GPU]implement sycl platform id #15904

Closed
wants to merge 3 commits into from
Closed

[XLA:GPU]implement sycl platform id #15904

wants to merge 3 commits into from

Conversation

mayuyuace
Copy link
Contributor

No description provided.

@mayuyuace mayuyuace changed the title implement sycl platform id [XLA:GPU]implement sycl platform id Aug 9, 2024
@NaiyerRizz NaiyerRizz added the kokoro:force-run Forces CI to rerun label Aug 9, 2024
@NaiyerRizz NaiyerRizz self-assigned this Aug 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Forces CI to rerun label Aug 9, 2024
@Zantares Zantares mentioned this pull request Aug 9, 2024
Draft
@penpornk penpornk self-requested a review August 9, 2024 07:32
@sgerrard
Copy link
Contributor

@penpornk A reminder that your review is requested on this PR.

@dimitar-asenov
Copy link
Member

@penpornk Could you please take a look at this PR?

penpornk
penpornk approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! PR looks good to me.

@mayuyuace
Copy link
Contributor Author

Sorry, I cannot see the details of feedback/copybara.

@dimitar-asenov
Copy link
Member

The internal checks are failing because the changes in this PR are not tested. Please add a test.

copybara-service bot pushed a commit that referenced this pull request Sep 25, 2024
@mayuyuace @Google-ML-Automation
PR #15904: [XLA:GPU]implement sycl platform id
5feee01 
Imported from GitHub PR #15904

Copybara import of the project:

--
df9b82a by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e108
PiperOrigin-RevId: 678590877
@copybara-service copybara-service bot mentioned this pull request Sep 25, 2024
Merged
@mayuyuace
Copy link
Contributor Author

mayuyuace commented Sep 25, 2024 •
edited
Loading

@dimitar-asenov
I want to know: I just modified some code in the original file, why do I need to add a new test?
Is it related to this pr: 11425. As what Shengyang1 said, "Currently, the code can only be built but not fully run since it relies on symbols(GpuDriver/GpuExecutor) which should be covered in other PRs.". So the file sycl_platform.h was not tested?

copybara-service bot pushed a commit that referenced this pull request Sep 25, 2024
@mayuyuace @Google-ML-Automation
PR #15904: [XLA:GPU]implement sycl platform id
3e05fdc 
Imported from GitHub PR #15904

Copybara import of the project:

--
df9b82a by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e108
PiperOrigin-RevId: 678590877
@dimitar-asenov
Copy link
Member

@mayuyuace As a general rule: any code modification (except for pure refactorings that do not change behavior) requires a test change that verifies the correctness of the change.

In this case, we can make an exception. But please do add tests as soon as possible.

copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Sep 25, 2024
@mayuyuace @tensorflower-gardener
PR #15904: [XLA:GPU]implement sycl platform id
f9dd25c 
Imported from GitHub PR openxla/xla#15904

Copybara import of the project:

--
df9b82ad0c35cb3f8ad8253b20a38a74f9318d73 by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f61eed4f729d0e5800401fb26da8693a06 by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e1084f3f9fa7338bae947dc258330707cbe52
PiperOrigin-RevId: 678590877
@copybara-service copybara-service bot mentioned this pull request Sep 25, 2024
Merged
@mayuyuace
Copy link
Contributor Author

@mayuyuace As a general rule: any code modification (except for pure refactorings that do not change behavior) requires a test change that verifies the correctness of the change.

In this case, we can make an exception. But please do add tests as soon as possible.

Thank you!

copybara-service bot pushed a commit that referenced this pull request Sep 25, 2024
@mayuyuace @Google-ML-Automation
PR #15904: [XLA:GPU]implement sycl platform id
a26b30b 
Imported from GitHub PR #15904

Copybara import of the project:

--
df9b82a by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e108
PiperOrigin-RevId: 678590877
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Sep 25, 2024
@mayuyuace @tensorflower-gardener
PR #15904: [XLA:GPU]implement sycl platform id
b46b168 
Imported from GitHub PR openxla/xla#15904

Copybara import of the project:

--
df9b82ad0c35cb3f8ad8253b20a38a74f9318d73 by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f61eed4f729d0e5800401fb26da8693a06 by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e1084f3f9fa7338bae947dc258330707cbe52
PiperOrigin-RevId: 678590877
copybara-service bot pushed a commit that referenced this pull request Sep 25, 2024
@mayuyuace @Google-ML-Automation
PR #15904: [XLA:GPU]implement sycl platform id
2c27c0f 
Imported from GitHub PR #15904

Copybara import of the project:

--
df9b82a by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e108
PiperOrigin-RevId: 678590877
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Sep 25, 2024
@mayuyuace @tensorflower-gardener
PR #15904: [XLA:GPU]implement sycl platform id
648b87e 
Imported from GitHub PR openxla/xla#15904

Copybara import of the project:

--
df9b82ad0c35cb3f8ad8253b20a38a74f9318d73 by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f61eed4f729d0e5800401fb26da8693a06 by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#15904 from Intel-tensorflow:qiming/implement_sycl_id d43e1084f3f9fa7338bae947dc258330707cbe52
PiperOrigin-RevId: 678590877
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Sep 25, 2024
@mayuyuace @tensorflower-gardener
PR #15904: [XLA:GPU]implement sycl platform id
b47eb03 
Imported from GitHub PR openxla/xla#15904

Copybara import of the project:

--
df9b82ad0c35cb3f8ad8253b20a38a74f9318d73 by mayuyuace <[email protected]>:

implement sycl platform id

--
72cf11f61eed4f729d0e5800401fb26da8693a06 by mayuyuace <[email protected]>:

remove override' of GetUncachedExecutor

Merging this change closes #15904

PiperOrigin-RevId: 678642780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@penpornk penpornk penpornk approved these changes

Assignees

@NaiyerRizz NaiyerRizz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mayuyuace @sgerrard @dimitar-asenov @penpornk @kokoro-team @NaiyerRizz