Parameter vs. buffer

[f0k]

The implementations currently use nn.Parameter to store precomputed values such as the window function or filterbank. I think they should be buffers instead, registered via model.register_buffer(), so they can be Tensors instead of Variables.

In fact, they should neither be Parameters nor buffers, because they should not become part of the state_dict, but PyTorch does not support this directly. You'd either make them a normal attribute and override _apply() so they are moved between devices correctly, or make them a buffer and override state_dict so they're not included in it.

[keunwoochoi]

Hey, thanks for letting us know about it. I liked nn.Parameter because it then deals with the device automatically. You're right though that nn.Parameter assumes Tensors, although by passing , requires_grad=False there's no issue on computational efficiency. Being part of state_dict, a remaining (small) issue would be then it using more disk space.

Overriding state_dict sounds a bit hassle for me (is it?). Could you explain how can we use _apply()?

[f0k]

I liked nn.Parameter because it then deals with the device automatically.

Yes, same for buffers.

Being part of state_dict, a remaining (small) issue would be then it using more disk space.

That, and if we ever want to change the implementation to compute the forward pass in another way (e.g., using a sparse matrix multiplication for the mel filterbank), we'd need to override _load_state_dict() to ignore the old parameter/buffer on loading.

Overriding state_dict sounds a bit hassle for me (is it?).

I think it would be something like:

def state_dict(self):
    result = super(ModuleName, self).state_dict()
    for k in ('window', 'whateverelse'):
        del result[k]

You could also register a hook, but I find it weird to register a hook when we're controlling the class? It's also a bit ugly in that you need to hard-code the names of the buffers you don't want to be included.

Could you explain how can we use _apply()?

I did it like this:

def _apply(self, fn):
    result = super(ModuleName, self)._apply(fn)
    # apply fn to all tensors we use as cached constants
    for k, v in result.__dict__.items():
       if torch.is_tensor(v):
           setattr(result, k, fn(v))
    return result

This will ensure to convert all attributes that are plain torch tensors (not variables) on a call to cuda() or half() or double() etc. Again, this is something that should be supported by PyTorch directly...

[f0k]

This will ensure to convert all attributes that are plain torch tensors (not variables) on a call to cuda() or half() or double() etc.

Bummer: This breaks compatibility with torch.nn.DataParallel. torch.nn.parallel.replicate does not call .to(), but explicitly copies only the parameters and buffers using a more performant way. So we should use buffers and override state_dict().