-
Notifications
You must be signed in to change notification settings - Fork 19.5k
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 tracking issue with lazy filled attributes #20783
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20783 +/- ##
==========================================
- Coverage 82.01% 82.00% -0.02%
==========================================
Files 555 555
Lines 51856 51874 +18
Branches 8021 8031 +10
==========================================
+ Hits 42530 42539 +9
- Misses 7377 7382 +5
- Partials 1949 1953 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -224,6 +224,27 @@ def build_wrapper(*args, **kwargs): | |||
with obj._open_name_scope(): | |||
obj._path = current_path() | |||
original_build_method(*args, **kwargs) | |||
# Check for any untracked attrs/vars | |||
if obj._tracker._has_untracked_attrs: | |||
if backend.backend() == "tensorflow": |
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.
Why is it backend specific?
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.
Only TF has this attr _tracked
which will have only tracked attrs to iterate. To put in numbers other backend will have atleast 50-55 additional attrs to iterate. Since this might be a rare case and if it is OK we can keep a single check ignoring TF specific.
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.
I don't quite get it -- can you explain in more detail?
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.
The attribute for layer, _tracked
is only initiated in TFLayer and hence only available for TF backend. It will log only that are initialized in custom layer
, which is what we interested for this case, along with few common attrs '_inbound_nodes', '_outbound_nodes', '_losses', '_loss_ids', '_losses_override'
.
Consider below custom layer:
class NGLayer(keras.layers.Layer):
def __init__(self,**kwargs):
super().__init__(**kwargs)
def build(self, input_shape):
self.l1 = []
for i in range(2):
l2 = []
self.l1.append(l2)
for j in range(2):
l2.append(keras.layers.Dense(1, name=f'dense_{i}_{j}'))
# print("before appending l2 to l1 in OK way")
# self.l1.append(l2) #This works
def call(self, x):
for l in self.l1:
for d in l:
x = d(x)
return x
The final list of obj._tracked
will be something like the below:
['_inbound_nodes', '_outbound_nodes', '_losses', '_loss_ids', '_losses_override', 'l1']
where 'l1
' being initiated in the custom layer by the user which was not tracked properly.
For other backends the attribute _tracked
is not available and hence we need to iterate through complete attributes initialized by Keras on the particular layer object to get the required attribute that missed tracking.
For TF backend the untracked attribute can be quickly retrieved using obj._tracked
list. However we can also get this attribute using __dict__.keys()
but list to iterate is big.
Currently there is an issue with Tracking when for custom layers the attributes initialized empty and assigned with empty values that filled at later stage. This PR addresses the same.
Fixes #20598