Skip to content

Spectral norm improvements#8590

Merged
ssnl merged 3 commits intopytorch:masterfrom
t-vi:spectral_norm_improvement
Jun 24, 2018
Merged

Spectral norm improvements#8590
ssnl merged 3 commits intopytorch:masterfrom
t-vi:spectral_norm_improvement

Conversation

@t-vi
Copy link
Collaborator

@t-vi t-vi commented Jun 17, 2018

@ssnl is this about what you had in mind?

I have to add tests. What would be resonable here

  • test that three calls in eval mode give the exact same thing and after remove
  • test convergence of SN in the right dimension using fixed random weights for Linear and ConvTranspose2d?

- Don't do iterations on weight in eval mode
  To facilitate this, register weight as buffer in order to be able
  to use module with spectral norm in eval mode after immediately
  after loading state dict (pytorch#8208)
- Use weight instead of weight_orig as weight when removing
  spectral norm
- Add dim parameter in case the normalization should occur w.r.t.
  a dimension other than 0 (pytorch#7865)
delattr(module, self.name + '_u')
delattr(module, self.name + '_orig')
module.register_parameter(self.name, weight)
module.register_parameter(self.name, torch.nn.Parameter(weight))

This comment was marked as off-topic.

This comment was marked as off-topic.

@ssnl
Copy link
Collaborator

ssnl commented Jun 18, 2018

Code looks reasonable to me. Thanks!

I think for the tests you proposed, 1 looks good. It might be hard to do a convergence test for 2. How about just test that _u has correct length? :)

@t-vi
Copy link
Collaborator Author

t-vi commented Jun 19, 2018

Thanks! Here are the tests.

@t-vi
Copy link
Collaborator Author

t-vi commented Jun 19, 2018

@ezyang The failures are a timeout and a strange jit fault in LSTM (which I didn't get near to, I hope), also the caffee rocm seems to have completed, but it is not recognized as completed (the latter seems common today)... Do you have any advice for me?

test/test_nn.py Outdated
self.assertTrue(hasattr(m, 'weight'))
self.assertTrue('weight' in m._parameters)

def test_spectral_norm_eval(self):

This comment was marked as off-topic.

Thank you, Simon, for the suggestions.
@t-vi
Copy link
Collaborator Author

t-vi commented Jun 24, 2018

"Aborted by user anonymous"?

@ssnl
Copy link
Collaborator

ssnl commented Jun 24, 2018

@pytorchbot retest this please

@ssnl
Copy link
Collaborator

ssnl commented Jun 24, 2018

@t-vi lol CI probably had some problem then. I restarted CI on this. Sorry!

Copy link
Collaborator

@ssnl ssnl left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @t-vi !

@ssnl ssnl merged commit fc22bf3 into pytorch:master Jun 24, 2018
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.

5 participants