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.

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

Revert "[Datasets] Add support for slicing Arrow blocks that contain tensor columns." #19517

Merged

Conversation

xwjiang2010
Copy link
Contributor

Reverts #19494

tune_linear_dataset_example is not happy.

@xwjiang2010
Copy link
Contributor Author

@clarkzinzow

Copy link
Contributor

@ericl ericl left a comment

Choose a reason for hiding this comment

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

Hmm sorry about this breakage. We can wait for verifying the GPU tests to pass before merging this revert.

@ericl ericl self-assigned this Oct 19, 2021
@matthewdeng
Copy link
Contributor

For some additional context, it appears that this is causing a change in the performance which leads the GPU tests to time out (not error out).

Before: https://buildkite.com/ray-project/ray-builders-branch/builds/4048#4dd2af8e-9267-45bb-84cf-5e5abd041c4c
After: https://buildkite.com/ray-project/ray-builders-branch/builds/4049#0d32e4f9-f78c-4046-898e-084a3e8490ef

test_gpu execution time increased from 266s to 451s (within threshold) while tune_linear_dataset_example went from 170.7s to exceeding the 300s timeout.

@ericl
Copy link
Contributor

ericl commented Oct 19, 2021

I see. then, it is likely this is since the patch was incorrect, and not actually causing a copy of the data (that might have been why I didn't go with .to/from pandas in the first place @clarkzinzow ).

@ericl ericl merged commit a6f9c93 into master Oct 19, 2021
@ericl ericl deleted the revert-19494-datasets/fix/tensor-column-block-slice-copy branch October 19, 2021 18:35
clarkzinzow added a commit to clarkzinzow/ray that referenced this pull request Oct 20, 2021
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.

None yet

3 participants