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

Make cache support methods #1145

Open
brandonwillard opened this issue Sep 12, 2024 · 1 comment
Open

Make cache support methods #1145

brandonwillard opened this issue Sep 12, 2024 · 1 comment

Comments

@brandonwillard
Copy link
Member

brandonwillard commented Sep 12, 2024

Currently, the cache function doesn't truly support methods (see #1129). Let's consider

  • making the cache interface private/internal,
  • adding support for this (and admitting that it's a public interface/utility), or—at the very least
  • adding a warning to prevent confusion.
@brandonwillard
Copy link
Member Author

brandonwillard commented Sep 12, 2024

I noticed from the helpful test case in #1129—and the relevant vllm code—that the cache decorator is being directly applied to the function object, so we may not even be able to tell when a function is a method in order to give a warning. We might be able to handle the case of a method descriptor object, but that wouldn't have helped in the examples above.

In other words, there's probably not a general way to warn people that they could be doing something undesirable for caching. Because of that, we might want to make the caching utilities private and leave it up to the user to determine how caching should work, since all we can really guarantee is that underlying objects are serializable in a way that can work for caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants