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

fix tabulate on norm wrappers #3772

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Conversation

chiamp
Copy link
Collaborator

@chiamp chiamp commented Mar 20, 2024

Resolves #3735.

When using map_variables, calls contains _CallInfo objects where the method does not exist in the module, causing the AttributeError seen in #3735. This PR adds a check to see if the method exists in the module.

This is a partial fix meant to unblock the current issue. However we still need to address the underlying issue on why there seems to be _CallInfo objects with methods that do not exist in their corresponding modules being added to the _DynamicContext. See #3779.

@chiamp chiamp self-assigned this Mar 20, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 59.48%. Comparing base (8220154) to head (4f561be).
Report is 2 commits behind head on main.

Files Patch % Lines
flax/linen/summary.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3772      +/-   ##
==========================================
- Coverage   59.49%   59.48%   -0.02%     
==========================================
  Files         101      101              
  Lines       12623    12627       +4     
==========================================
+ Hits         7510     7511       +1     
- Misses       5113     5116       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chiamp chiamp requested a review from cgarciae March 20, 2024 00:19
@@ -460,7 +460,7 @@ def _get_variables():
call_depth = len(c.path)
inputs = _process_inputs(c.args, c.kwargs)

if c.path in visited_paths:
if c.path in visited_paths or not hasattr(c.module, c.method):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what is happening here? Why c doesn't have module and method?

@copybara-service copybara-service bot merged commit c53a515 into google:main Mar 22, 2024
19 checks passed
@chiamp chiamp deleted the tabulate_norm branch March 22, 2024 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when calling module tabulate involving WeightNorm
3 participants