Skip to content

[Data] Add iter_rows to DatasetIterator - #33180

Merged
amogkam merged 4 commits into
ray-project:masterfrom
amogkam:huggingface-deprecation-warning
Mar 11, 2023
Merged

[Data] Add iter_rows to DatasetIterator#33180
amogkam merged 4 commits into
ray-project:masterfrom
amogkam:huggingface-deprecation-warning

Conversation

@amogkam

@amogkam amogkam commented Mar 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

Why are these changes needed?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@amogkam
fix
8c0c412 
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
@amogkam amogkam changed the title [AIR/Data] Fix session.get_dataset_shard user warnings in HuggingfaceTrainer [Data] Add iter_rows to DatasetIterator Mar 9, 2023
@amogkam
lint
158021d 
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
bveeramani
bveeramani reviewed Mar 10, 2023
View reviewed changes
Comment thread python/ray/data/_internal/pipelined_dataset_iterator.py Outdated
Comment thread python/ray/data/_internal/pipelined_dataset_iterator.py
Comment thread python/ray/data/dataset_iterator.py
Comment thread python/ray/data/dataset_iterator.py Outdated
Comment thread python/ray/data/tests/test_dataset_iterator.py Outdated
Comment thread python/ray/data/tests/test_dataset_iterator.py
@ericl ericl assigned c21 and unassigned ericl Mar 10, 2023
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Mar 10, 2023
@amogkam
comments
c866296 
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
@amogkam amogkam removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Mar 10, 2023
@amogkam
2
7e83161 
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
jianoaix
jianoaix reviewed Mar 10, 2023
View reviewed changes
Comment thread python/ray/data/_internal/dataset_iterator_impl.py
# During row-based ops, we also choose a batch format that lines up with the
# current dataset format in order to eliminate unnecessary copies and type
# conversions.
ctx = DatasetContext.get_current()

@jianoaix jianoaix Mar 10, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This part is duplicating code of Dataset.iter_rows(). Should we just call it or make Dataset.iter_rows() call this one?

@amogkam amogkam Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This PR already changes it so that dataset.iter_rows() calls into this: https://github.com/ray-project/ray/pull/33180/files#diff-5a812d24fc3efd599284bb40dcbd2c6627bbe324e1c89a6e623b4dccf8e5de44R2858

Following the same patter we do for iter_batches

@amogkam
amogkam merged commit ad78454 into ray-project:master Mar 11, 2023
@amogkam
amogkam deleted the huggingface-deprecation-warning branch March 11, 2023 01:26
@c21 c21 mentioned this pull request Mar 13, 2023
Merged
7 tasks
ericl pushed a commit that referenced this pull request Mar 14, 2023
@c21
[Datasets] Refactor iter_rows() for DatasetIterator classes (#33257)
a3b89f6 
This is to fix CI test failure introduced in #33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.


<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request Mar 21, 2023
@amogkam @ProjectsByJackHe
[Data] Add iter_rows to DatasetIterator (ray-project#33180)
2ee8ebd 
The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

---------

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: Jack He <jackhe2345@gmail.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request Mar 21, 2023
@c21 @ProjectsByJackHe
[Datasets] Refactor iter_rows() for DatasetIterator classes (ray-proj…
a9471b2 
…ect#33257)

This is to fix CI test failure introduced in ray-project#33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.

<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">

Signed-off-by: Jack He <jackhe2345@gmail.com>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@amogkam @edoakes
[Data] Add iter_rows to DatasetIterator (ray-project#33180)
b0b281b 
The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

---------

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@c21 @edoakes
[Datasets] Refactor iter_rows() for DatasetIterator classes (ray-proj…
bb36746 
…ect#33257)

This is to fix CI test failure introduced in ray-project#33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.

<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
@amogkam @peytondmurray
[Data] Add iter_rows to DatasetIterator (ray-project#33180)
6ea5340 
The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

---------

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
@c21 @peytondmurray
[Datasets] Refactor iter_rows() for DatasetIterator classes (ray-proj…
246f41b 
…ect#33257)

This is to fix CI test failure introduced in ray-project#33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.


<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@amogkam @elliottower
[Data] Add iter_rows to DatasetIterator (ray-project#33180)
0972d0e 
The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

---------

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: elliottower <elliot@elliottower.com>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@c21 @elliottower
[Datasets] Refactor iter_rows() for DatasetIterator classes (ray-proj…
93b155a 
…ect#33257)

This is to fix CI test failure introduced in ray-project#33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.

<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
@amogkam @ProjectsByJackHe
[Data] Add iter_rows to DatasetIterator (ray-project#33180)
30dd234 
The iter_rows API that HuggingfaceTrainer uses internally is not available in DatasetIterator, which falls back to the base dataset and outputs User warnings about the change to DatasetIterator.

This PR adds an iter_rows API to DatasetIterator so we don't fallback to the base dataset.

---------

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: Jack He <jackhe2345@gmail.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
@c21 @ProjectsByJackHe
[Datasets] Refactor iter_rows() for DatasetIterator classes (ray-proj…
6b3b943 
…ect#33257)

This is to fix CI test failure introduced in ray-project#33180 ([example of buildkite failure](https://buildkite.com/ray-project/oss-ci-build-branch/builds/2690#0186da0a-17c7-40dd-a43c-1de36c4894f4)). We need to add `iter_rows()` method for StreamSplitDatasetIterator as well.

<img width="1848" alt="Screen Shot 2023-03-13 at 11 24 05 AM" src="https://user-images.githubusercontent.com/4629931/224795253-ecf4e362-4253-4d60-80ff-f87e138ef773.png">

Signed-off-by: Jack He <jackhe2345@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bveeramani bveeramani bveeramani left review comments
@jianoaix jianoaix jianoaix approved these changes
@ericl ericl Awaiting requested review from ericl
@scv119 scv119 Awaiting requested review from scv119
@clarkzinzow clarkzinzow Awaiting requested review from clarkzinzow
@jjyao jjyao Awaiting requested review from jjyao
@c21 c21 Awaiting requested review from c21

Assignees

@ericl ericl

@c21 c21

@bveeramani bveeramani

@jianoaix jianoaix

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@amogkam @ericl @bveeramani @jianoaix @c21