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

[docs] batch inference docs #34567

Merged
merged 60 commits into from
May 2, 2023
Merged

[docs] batch inference docs #34567

merged 60 commits into from
May 2, 2023

Conversation

maxpumperla
Copy link
Contributor

@maxpumperla maxpumperla commented Apr 19, 2023

Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla maxpumperla requested a review from a team as a code owner April 19, 2023 12:44
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@amogkam amogkam self-assigned this Apr 19, 2023
amogkam

This comment was marked as resolved.

Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla maxpumperla changed the title [WIP][docs] batch inference docs [docs] batch inference docs Apr 25, 2023
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>


# __hf_quickstart_prediction_start__
scale = ray.data.ActorPoolStrategy(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
scale = ray.data.ActorPoolStrategy(2)
scale = ray.data.ActorPoolStrategy(size=2)

Kwargs will be required in strict mode for actor pool strategy.

ndarrays (``Dict[str, np.ndarray]``), with each key-value pair representing a column
in the table.

* **Tensor datasets** (single-column): Each batch will be a single
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be disallowed in strict mode.

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.

The strict mode by default PR is ahead of schedule--- we can likely merge by end of today. Given this, how about making the new doc directly for the new API?

maxpumperla and others added 4 commits April 29, 2023 09:50
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
Copy link
Contributor Author

@ericl the reason is that we wanted to merge this soon, and then follow up with the latest changes. I think it's valuable to have a 2.4 snapshot and then apply the delta of the API changes. That's what we agreed on in the Data standup. If you insist, I can make the changes now.

@pcmoritz @zhe-thoughts any thoughts?

@zhe-thoughts
Copy link
Collaborator

@ericl @pcmoritz @maxpumperla @amogkam : Yes I think let's merge this one first and have a follow-up to reflect changes for strict mode. Not a big deal, but structurally it's easier to understand

doc/source/_toc.yml Outdated Show resolved Hide resolved
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.

Sounds pretty good. Can you address the TOC order change though? That is my only blocking comment.

@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 May 1, 2023
Copy link
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

LGTM, let's do the strict mode simplifications as a follow up. It will get a bunch simpler 🥳

Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@krfricke krfricke merged commit 1564c51 into master May 2, 2023
@krfricke krfricke deleted the mp_batch_inference_docs branch May 2, 2023 08:59
architkulkarni pushed a commit to architkulkarni/ray that referenced this pull request May 16, 2023
Preview: https://anyscale-ray--34567.com.readthedocs.build/en/34567/data/batch_inference.html

Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants