[misc][data.llm] Generalize the builder pattern in ray.data.llm#58484
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors build_llm_processor to the more generic build_processor. This is a good change that makes the API more flexible for various processing workloads beyond just LLM inference. The renaming has been applied consistently and thoroughly across the entire codebase, including source files, documentation, examples, and tests. The changes are well-executed and I found no issues in my review.
|
Will wait for #58298 to finalize before deciding if this PR is still necessary. |
bd646b6 to
9272819
Compare
nrghosh
left a comment
There was a problem hiding this comment.
conflicts resolved, good for merge
Head branch was pushed to by a user without write access
46dbc11 to
8787f45
Compare
|
Rebase to address failing doc tests. |
nrghosh
left a comment
There was a problem hiding this comment.
cc @kouroshHakha ready to merge 🚀
python/ray/data/llm.py
Outdated
| """ | ||
| [DEPRECATED] Prefer build_processor. Build a LLM processor using the given config. | ||
| """ | ||
| deprecation_warning( |
There was a problem hiding this comment.
wait this should be a decorator and we should also remove the PublicAPI annotation from build_llm_processor
There was a problem hiding this comment.
I see similar usages of directly using the deprecation_warning helper: https://github.com/search?q=repo%3Aray-project%2Fray+%22deprecation_warning%28%22&type=code&p=2. I'm not able to find associated decorators though.
There was a problem hiding this comment.
Will remove the publicAPI annotation.
There was a problem hiding this comment.
oh I guess I was talking about Deprecated class:
ray/python/ray/_common/deprecation.py
Line 61 in b8f22e9
Maybe we can use that instead?
There was a problem hiding this comment.
ah yeah, this is neater. adjusted in my latest revision.
Signed-off-by: jeffreyjeffreywang <jeffjeffreywang@gmail.com>
Signed-off-by: jeffreyjeffreywang <jeffjeffreywang@gmail.com>
Signed-off-by: jeffreyjeffreywang <jeffjeffreywang@gmail.com>
Signed-off-by: jeffreyjeffreywang <jeffjeffreywang@gmail.com>
8787f45 to
405fb1f
Compare
|
Rebasing onto latest master |
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
…project#58484) Signed-off-by: peterxcli <peterxcli@gmail.com>
Description
As discussed in https://docs.google.com/document/d/1danbyJjd3Zl_Q-CSsS3PjxtG4K9dZkyn0A9t7i4Fyjg/edit?disco=AAABtNCDbfw, the current builder function
build_llm_processoris overly specific to LLM inference workloads and not flexible enough to support additional processors, such as those for multimodal preprocessing. To address this, we’ve generalized it tobuild_processorto better accommodate a broader range of LLM-related workloads.Related issues
N/A
Additional information
Original:
Updated: