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

C implementation of LSTM #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

nkoumchatzky
Copy link
Collaborator

Pure C implementation for LSTMs with basic LSTM cell, including pre-multiplication and CUDA-like input format [(sum(T_i)) X outputSize], where T_i are the number of timesteps for each element of the batch, in decreasing value.

@nkoumchatzky nkoumchatzky force-pushed the nkoumchatzky/clstm branch 2 times, most recently from c4ff053 to d225b51 Compare June 3, 2017 00:59
* New "sparse/size" representation
* Full LSTM in C
* VSeqLSTM to wrap this data representation + C implementation
* Augmentation of the VariableLength decorator with this data representation from an array of tensors
* unit tests
* speed tests
@nkoumchatzky nkoumchatzky changed the title [WIP] - C implementation of LSTM C implementation of LSTM Jun 4, 2017
Copy link
Collaborator

@mirandaconrado mirandaconrado left a comment

Choose a reason for hiding this comment

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

The code itself looks good in general. There are lots of small changes that I think should be made either for correctness or clarity.

typedef int THInteger_t;
typedef void THRNNState;

#define THRNN_resizeAs_indices(I1, I2) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put the code in the macro between brackets for safety.

#include <omp.h>
#endif

/* Set tensor->size[0] MACRO */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not only first position.

#define THRNN_LSTM_SET_SIZE(t, dim, newSize) ( t->size[dim] = newSize )
#endif

/* Set tensor->size[0] MACRO */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not only first and stride.

// and a LongStorage of default sizes for new individual buffers
struct THRNN_(buffer)* THRNN_(create_buffer)(THTensor* buf, THLongStorage* default_buffer_sizes)
{
THTensor** arr;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused.

buffer->sizes = (int*)realloc(buffer->sizes, buffer->len * sizeof(int));
buffer->sizes[buffer->len-1] = size;
buffer->array = (THTensor**)realloc(buffer->array, buffer->len * sizeof(THTensor**));
THTensor* new_guy = THTensor_(new)();
Copy link
Collaborator

Choose a reason for hiding this comment

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

new_guy!

self.cgradInput = self.cgradInput:type(first_input:type())
self._input = self._input or first_input.new()
self._input = self._input:type(first_input:type())
self._input = self._input or {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

self._input will always be defined as a tensor in the last line

local input = torch.Tensor(seqlen, batchsize, inputsize)
local gradOutput = torch.Tensor(seqlen, batchsize, outputsize)
local input = torch.Tensor(seqlen, batchsize, inputsize):uniform()
local gradOutput = torch.Tensor(seqlen, batchsize, outputsize):uniform()

local t = torch.Timer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

t as replaced by tic/toc

-- main test
collectgarbage()
t:reset()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove

@@ -2828,6 +2828,110 @@ function rnntest.SeqLSTM_Lua_vs_C()
end
end

function rnntest.SeqLSTM_vs_VSeqLSTM()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This benchmark is duplicated above, except that one uses double and the other float.

if not testLM then
for i=1,batchSize do
input[i] = torch.randn(torch.random(1,maxLength), hiddenSize)
input[i] = torch.randn(i,hiddenSize)--torch.random(1,maxLength), hiddenSize)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove comment.

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.

3 participants