Err in char_rnn tutorial

[williamFalcon]

@apaszke
This tutorial "char_rnn_classification" has a bug in the forward part of this code:

import torch.nn as nn
from torch.autograd import Variable

class RNN(nn.Module):
    def __init__(self, input_size, hidden_size, output_size):
        super(RNN, self).__init__()

        self.hidden_size = hidden_size

        self.i2h = nn.Linear(input_size + hidden_size, hidden_size)
        self.i2o = nn.Linear(input_size + hidden_size, output_size)
        self.softmax = nn.LogSoftmax(dim=1)

    def forward(self, input, hidden):
        combined = torch.cat((input, hidden), 1)
        hidden = self.i2h(combined)
        output = self.i2o(combined)
        output = self.softmax(output)
        return output, hidden

    def initHidden(self):
        return Variable(torch.zeros(1, self.hidden_size))

n_hidden = 128
rnn = RNN(n_letters, n_hidden, n_categories)

The RNN formula is:

Which is implemented by these lines:

        combined = torch.cat((input, hidden), 1)
        hidden = self.i2h(combined)

However, these lines:

        output = self.i2o(combined)   
        output = self.softmax(output)

Are trying to project into the classification space. However, the self.i2o operates on the combined output instead of the ht output.

This implementation uses the wrong formula:

But the correct formula is:

Which can be implemented as:

    def forward(self, input, hidden):
        combined = torch.cat((input, hidden), 1)
        hidden = self.i2h(combined)
        output = self.i2o(hidden) # this line changed   (bc hidden = ht. combined = ht-1)
        output = self.softmax(output)
        return output, hidden

Basically, the current implementation does this:

But it really should do this:

[ayush1999]

@williamFalcon Small question. You mentioned that

combined = torch.cat((input, hidden), 0)
hidden = self.i2h(combined)

Performs the RNN formula. I didn't get how concatenating the input and hidden tensors is useful in this scenario. Why the need for concatenation?

[EmreOzkose]

I think it should be changed. The implemented formula is wrong?

[Lguyogiro]

@williamFalcon Small question. You mentioned that

combined = torch.cat((input, hidden), 0)
hidden = self.i2h(combined)

Performs the RNN formula. I didn't get how concatenating the input and hidden tensors is useful in this scenario. Why the need for concatenation?

I know this is an old issue/comment....but you don't "need" to concatenate the input and hidden tensors, this is just a way to calculate the same thing without having two separate weights tensors (the standard formulae typically assume two weights matrices, see below)

Also I just wanted to chime in. I agree that this should be changed.

[mikebrow]

/assigntome