[serve.llm] delete dead code from prompt format days#53621
[serve.llm] delete dead code from prompt format days#53621kouroshHakha merged 11 commits intoray-project:masterfrom
Conversation
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
07a9a52 to
0ca98dd
Compare
c600add to
f415836
Compare
- Merge branch 'master' into delete_abstraction
f415836 to
d8d36ec
Compare
|
@nrghosh you can add |
kouroshHakha
left a comment
There was a problem hiding this comment.
more stuff can be deleted in this pr.
There was a problem hiding this comment.
The huggingfacePromptFormat should also be deleted and absorbed? There is basically no need for this abstraction anymore?
There was a problem hiding this comment.
if you search for generate_prompt I don't think there is any place that uses it. We need to do a holistic clean up here. Most of items in this file are dead code.
There was a problem hiding this comment.
yes, generate_prompt isn't used in other places but the class is just used as a format specifier - mostly in tests. If we get rid of all instances there won't be the same pydantic type safety / standardization (if that becomes something that needs to be tested / guaranteed later).
But for now, yes can get rid of most of this code - but will need to get rid of some tests as well (like in test_prompt_formats and format specifiers in server_models.py (part of LLMConfig) (which could break some tests but is fixable).
Will review and see what else could be cleaned up.
There was a problem hiding this comment.
yes all of those can be deleted. the tests also need clean up.
…ll usages, clean up tests Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
8178cc0 to
060161c
Compare
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
kouroshHakha
left a comment
There was a problem hiding this comment.
great. just some nits:
|
|
||
| from ray.llm._internal.utils import try_import | ||
|
|
||
| if TYPE_CHECKING: |
There was a problem hiding this comment.
delete TYPE_CHECKING?
| ) | ||
| except OSError: | ||
| pass | ||
| # Note (Nikhil): prompt_format functionality has been removed |
There was a problem hiding this comment.
nit: no need to keep this note as it won't make sense anymore.
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Why are these changes needed?
Cleanup - quick todo
Related issue number
https://anyscale1.atlassian.net/browse/LLM-1705
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.