-
Notifications
You must be signed in to change notification settings - Fork 648
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
[nnx] cleanup CallableProxy #3608
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3608 +/- ##
=======================================
Coverage 56.70% 56.70%
=======================================
Files 100 100
Lines 12006 12005 -1
=======================================
Hits 6808 6808
+ Misses 5198 5197 -1 ☔ View full report in Codecov by Sentry. |
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.
There is also a CI failure
2606a77
to
ec8af91
Compare
Thanks! Fixed. |
ec8af91
to
b3c181d
Compare
@@ -330,10 +330,10 @@ def apply( | |||
accessesor = DelayedAccessor() |
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.
still some typos here: accessesor -> accessor
flax/experimental/nnx/nnx/module.py
Outdated
accessesor: DelayedAccessor, *args, **kwargs | ||
) -> tuple[tp.Any, M]: | ||
module = self.clone() | ||
fn = accessesor(module) |
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.
and here
b3c181d
to
44e96a2
Compare
What does this PR do?
CallableProxy
now create aDelayedAccessor
by default if no accessor is given.DelayedAccessors
at many callsitesCallableProxy
is used.