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

StringSplit operator #18016

Merged
merged 17 commits into from
Jan 12, 2024
Merged

StringSplit operator #18016

merged 17 commits into from
Jan 12, 2024

Conversation

adityagoel4512
Copy link
Contributor

Description

Motivation and Context

Closes #17596

@adityagoel4512 adityagoel4512 force-pushed the stringsplit branch 3 times, most recently from c29ba03 to 472424d Compare October 18, 2023 17:14
@adityagoel4512 adityagoel4512 marked this pull request as ready for review November 2, 2023 10:22
@adityagoel4512 adityagoel4512 mentioned this pull request Nov 10, 2023
Merged
yuslepukhin
yuslepukhin requested changes Dec 12, 2023
View reviewed changes
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/nn/string_split.cc Outdated Show resolved Hide resolved
@thiagocrepaldi
Copy link
Contributor

@adityagoel4512 any update on this? ORT 1.17 release is coming soon :)

@adityagoel4512
Copy link
Contributor Author

@adityagoel4512 any update on this? ORT 1.17 release is coming soon :)

I'm waiting on a review/remaining CI to be run 😅

@yuslepukhin
Copy link
Member

This GCC warning may still need to be addressed: #18016 (comment)

@yuslepukhin
Copy link
Member

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@yuslepukhin
Copy link
Member

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline,Android CI Pipeline

@yuslepukhin
Copy link
Member

/azp run iOS CI Pipeline,ONNX Runtime React Native CI Pipeline

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 10 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

yuslepukhin
yuslepukhin previously approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

:shipit:

@yuslepukhin
Copy link
Member

This needs a merge from main.

@yuslepukhin
Copy link
Member

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@yuslepukhin
Copy link
Member

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline,Android CI Pipeline

@yuslepukhin
Copy link
Member

/azp run iOS CI Pipeline,ONNX Runtime React Native CI Pipeline

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 10 pipeline(s).

yuslepukhin
yuslepukhin approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

:shipit:

@adityagoel4512
Copy link
Contributor Author

Could you retrigger the Linux CPU CI Pipeline (arm64_build Linux_py_Wheels_aarch64) CI @yuslepukhin? It looks like a spurious failure to me but I cannot check this easily.

@yuslepukhin
Copy link
Member

Could you retrigger the Linux CPU CI Pipeline (arm64_build Linux_py_Wheels_aarch64) CI @yuslepukhin? It looks like a spurious failure to me but I cannot check this easily.

Ignore that for now.

@yuslepukhin yuslepukhin merged commit c23410a into microsoft:main Jan 12, 2024
73 of 75 checks passed
mszhanyi pushed a commit that referenced this pull request Jan 15, 2024
@adityagoel4512 @mszhanyi
StringSplit operator (#18016)
7b6a615 
### Description
<!-- Describe your changes. -->



### Motivation and Context
Closes #17596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xadupre xadupre xadupre left review comments

@yuslepukhin yuslepukhin yuslepukhin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.17
Development

Successfully merging this pull request may close these issues.

Add compute kernel for StringSplit
8 participants
@adityagoel4512 @thiagocrepaldi @skottmckay @yuslepukhin @xadupre @justinchuby @YUNQIUGUO @jywu-msft