Skip to content

Conversation

@dominikgrewe
Copy link
Member

Allows modules that want to implement custom read and write methods to
call parent.read/write.

Allows modules that want to implement custom `read` and `write` methods to
call `parent.read/write`.
@nicholas-leonard
Copy link
Member

@dominikgrewe This won't break anything, but now all modules will be serialized through those methods you just created.

@dominikgrewe
Copy link
Member Author

Yes, but they're just doing what torch serialization would have done anyway (apart from filtering out non-writable objects).

@soumith
Copy link
Member

soumith commented Jan 22, 2016

looks pretty useful! Thanks.

soumith added a commit that referenced this pull request Jan 22, 2016
read and write methods for nn.Module.
@soumith soumith merged commit cfed114 into torch:master Jan 22, 2016
@soumith
Copy link
Member

soumith commented Jan 22, 2016

@nicholas-leonard this is useful when implementing custom read-write, you can change some fields and then just call parent.read /write

@eladhoffer
Copy link

@soumith can't we use this to avoid the need for #526 by adding an option to write just weight, bias and running_mean/var (empty tensors for all other fields)?

@dominikgrewe
Copy link
Member Author

There are various other things you might want to save though, e.g. the convolution settings (kernel size, stride, padding etc) for a convolution module. So we could only do without clearState if many modules had custom read & write functions. It's worth thinking about though. What other uses for clearState are there aside from serialization?

@eladhoffer
Copy link

@dominikgrewe yes, what I meant is removing the output, gradInput and buffer tensors. Other fields should be kept.

@soumith
Copy link
Member

soumith commented Jan 26, 2016

@eladhoffer not a good idea. if the intention is to save a model, it's to save the whole model as is. we can introduce an nn.save that calls clearState before hand, and that's okay.

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.

4 participants