-
Notifications
You must be signed in to change notification settings - Fork 91
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
more general setattribute #1541
Conversation
77b90e3
to
c9c444a
Compare
c9c444a
to
ee736c1
Compare
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.
Looks great! Let's add unit tests for dataclass and setattr in a follow-up PR (to avoid conflicts on the current PR)
@@ -2073,9 +2075,13 @@ def impl(fn, iterable, initializer, null): | |||
return _interpret_call(impl, fn, iterable, initializer, null) | |||
|
|||
|
|||
class ThunderInterpreterObject: |
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 should have a docstring here
@@ -2476,7 +2482,7 @@ class MutMappingWrapperMethods(WrappedValue): | |||
def __new__(cls, /, *args, **kwds): |
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 it be kwargs
Consult the wrappers in order to build return value. This gives us a more general setattribute, e.g. for HF static KVCache.
In pseudocode
So for example for the "existing" HF Static Cache, we have the following epilogue, note how the static cache (provided as an input) is inserted into the model output rather than trying to construct a new one (which was what it tried to do before).