-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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] [Draft] First pass of batch inference docs #34185
Conversation
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an approver but some suggestsions.
doc/source/batch-inference/doc_code/torch_image_batch_pretrained.py
Outdated
Show resolved
Hide resolved
dataset = dataset.map_batches(concatenate, batch_format="pandas") | ||
|
||
# Define the model class for prediction. | ||
class TorchModel: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is calling this a model the right thing? Should we call it a pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can rename to TorchPredictor. But I don't think pipeline is very intuitive.
doc/source/batch-inference/doc_code/pytorch_tabular_batch_prediction.py
Outdated
Show resolved
Hide resolved
from tensorflow import keras # this is needed for tf<2.9 | ||
from keras import layers | ||
|
||
self.model = keras.Sequential( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't model have a leading dunder?
doc/source/batch-inference/doc_code/torch_image_batch_pretrained.py
Outdated
Show resolved
Hide resolved
doc/source/batch-inference/doc_code/torch_image_batch_pretrained.py
Outdated
Show resolved
Hide resolved
|
||
|
||
Writing batch UDFs | ||
------------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we calling them UDF? That seems like Spark terminology. Why don't we call them stateful model classes or something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is common lingo in this context. My worry is rather that "UDF" and other acronyms might look intimidating to new users
The following is an example to make use of those transformation APIs for processing | ||
the Iris dataset. | ||
|
||
.. literalinclude:: ../data/doc_code/transforming_datasets.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also give an example of how to pass a custom constructor arg / custom call arg. It comes up often in feedback.
Model Inferencing | ||
================= | ||
|
||
Model inferencing involves applying a :meth:`ds.map_batches() <ray.data.Dataset.map_batches>` to our transformed dataset with a pre-trained model as a UDF. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this page, how about using phrasing more like custom model / Python class instead of stateful operations / stateful UDFs?
I think it's OK to use UDF terminology for Data docs more generally, since it's more accurate, but for this workflow custom models are probably what users are looking for.
num_cpus=8) | ||
|
||
|
||
**How should I deal with OOM errors due to heavy model memory usage?** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we also add a FAQ on how to configure batch size? (which may affect both performance and memory usage?)
|
||
|
||
Configuring Batch Size | ||
~~~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see you have it here. IMO, this page should be combined with the former, since preprocessing + inference are so tied together. Also, batch size applies for both preprocessing and inference.
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
@@ -14,6 +14,13 @@ parts: | |||
- file: ray-overview/ray-libraries | |||
title: "Ecosystem" | |||
|
|||
- file: batch-inference/getting-started |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good way to extend this to other CUJs would be to make our "Use Cases" section expandable and then have several cases such as this one there.
@@ -0,0 +1,44 @@ | |||
Scalable Offline Batch Inference | |||
================================ | |||
:ref:`Ray Data <datasets>` offers a highly performant and scalable solution for offline batch inference and processing on large amounts of data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick paragraph on what batch inference is maybe? Also, we should somehow mention that there are many ways to run batch inference with Ray, but we're starting with the most convenient here (or so).
|
||
Why should I use Ray for offline batch inference? | ||
------------------------------------------------- | ||
1. **Faster and Cheaper for modern Deep Learning Applications**: Ray Data is built for hybrid CPU+GPU workloads. Through streaming based execution, CPU tasks like reading and preprocessing can be executed concurrently with GPU inference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just "Ray" here? We don't know about "Data" at this point
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
|
This can be closed because of #34567 |
Initial pass of batch inference docs.
Open questions:
Why are these changes needed?
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.