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

[PyTorch] Fix module types (LazyLinear -> Linear) #2225

Merged
merged 1 commit into from Jul 31, 2022

Conversation

atgctg
Copy link
Contributor

@atgctg atgctg commented Jul 30, 2022

Description of changes:

In 6.3 Parameter Initialization, surprisingly LazyLinear modules have the type <class 'torch.nn.modules.linear.Linear'> during init, so the in-place updates in the PyTorch examples are skipped.

This PR fixes this by making the if statements check for the correct nn.Linear type.

Not applied:

def init_constant(module):
    if type(module) == nn.LazyLinear: # this is False
        nn.init.constant_(module.weight, 1)
        nn.init.zeros_(module.bias)
net.apply(init_constant)
net[0].weight.data[0], net[0].bias.data[0]
(tensor([ 0.1594,  0.9592, -1.4913,  0.8780]), tensor(0.))

Applied:

def init_constant(module):
    if type(module) == nn.Linear: # this is True
        nn.init.constant_(module.weight, 1)
        nn.init.zeros_(module.bias)
net.apply(init_constant)
net[0].weight.data[0], net[0].bias.data[0]
(tensor([1., 1., 1., 1.]), tensor(0.))

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

@atgctg atgctg changed the title Fix PyTorch module types (LazyLinear -> Linear) [PyTorch] Fix module types (LazyLinear -> Linear) Jul 30, 2022
@d2l-bot
Copy link
Member

d2l-bot commented Jul 30, 2022

Job d2l-en/PR-2225/1 is complete.
Check the results at http://preview.d2l.ai/d2l-en/PR-2225/

@d2l-bot
Copy link
Member

d2l-bot commented Jul 30, 2022

Job d2l-en/PR-2225/2 is complete.
Check the results at http://preview.d2l.ai/d2l-en/PR-2225/

@d2l-bot
Copy link
Member

d2l-bot commented Jul 30, 2022

Job d2l-en/PR-2225/3 is complete.
Check the results at http://preview.d2l.ai/d2l-en/PR-2225/

@d2l-bot
Copy link
Member

d2l-bot commented Jul 30, 2022

Job d2l-en/PR-2225/4 is complete.
Check the results at http://preview.d2l.ai/d2l-en/PR-2225/

Copy link
Member

@AnirudhDagar AnirudhDagar left a comment

Choose a reason for hiding this comment

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

Hi @atgctg thanks for the PR. Great catch with this bug, which was overlooked in #2038. It is actually not surprising for the lazy layers to be torch.nn.modules.linear.Linear type, since PyTorch devs designed the lazy modules intending to convert them to their non-lazy analogue after the first dry run (param init). See docs.

@AnirudhDagar AnirudhDagar merged commit 489ab4a into d2l-ai:master Jul 31, 2022
@astonzhang
Copy link
Member

@atgctg thanks. Could you send another PR to replace your github id with your name in our acknowledgement:
https://github.com/d2l-ai/d2l-en/blob/master/chapter_preface/index.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants