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

Relax parameter attribute check #706

Merged
merged 1 commit into from
Dec 4, 2020

Conversation

avital
Copy link
Contributor

@avital avital commented Dec 4, 2020

We currently try to enforce that a user can't do:

class MyModule(nn.Module):
  def setup(self):
    self.name1 = self.param('name2', ...)

But the check is brittle and entirely disallows assigning dicts or lists of parameters. This check relaxes our heuristic such that it doesn't run at all for lists or dicts of parameters assigned to attributes during setup.

See #705 (comment) for a bit more context.

We currently try to enforce that a user can't do:

```py
class MyModule(nn.Module):
  def setup(self):
    self.name1 = self.param('name2', ...)
```

But the check is brittle and entirely disallows assigning dicts
or lists of parameters. This check relaxes our heuristic such
that it doesn't run at all for lists or dicts of parameters
assigned to attributes during setup.

See google#705 (comment)
for a bit more context.
Copy link
Member

@jheek jheek left a comment

Choose a reason for hiding this comment

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

LGTM

@copybara-service copybara-service bot merged commit 9a0b06d into google:master Dec 4, 2020
@avital avital deleted the param-names branch December 4, 2020 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants