verne.freenode.net changed the topic of #mlpack to: http://www.mlpack.org/ -- We don't respond instantly... but we will respond. Give it a few minutes. Or hours. -- Channel logs: http://www.mlpack.org/irc/
govg has quit [Quit: leaving]
nilay has joined #mlpack
govg has joined #mlpack
Mathnerd314 has quit [Ping timeout: 264 seconds]
< nilay>
zoq: Hi, how are you?
< nilay>
zoq: I think there are a few bugs in conv_layer.hpp
< zoq>
nilay: Hello, can you provide a simple test case that shows the error?
< zoq>
nilay: About the changes in the pooling layer, have you tested it with stride > 1?
mentekid has quit [Ping timeout: 240 seconds]
sumedhghaisas has quit [Ping timeout: 264 seconds]
mentekid has joined #mlpack
nilay has joined #mlpack
< nilay>
zoq: i have updated the pooling_layer gist
< nilay>
it works for stride > 1 correctly.
< nilay>
also in the conv_layer previously we were not convolving with the correct weight slice
< nilay>
and the padding was not correct, we gave a padded output, but what was happening was the arma::Mat<eT> convOutput matrix was of smaller size than output.slice.. so it was not giving correct answer when i did convolution with an identitiy kernel
< nilay>
now when i perform padding in the forward pass, the answer is correct, the output is exactly same as input.
< zoq>
nilay: Also if you pad you should start with filterSize - 1 instead of filterSize. Here is a simple example the first matrix is the filter, the second matrix is the padded output of the convolution method and the last matrix is what I get from your padding: https://gist.github.com/zoq/4faf019e74854e3d84327484999385c5
< zoq>
If you multiply the filter with the first entries it's always zero, so we can just start with the second iteration.
< zoq>
nilay: Btw. nice catch with the wrong index: weights.slice(outMapIdx).
< zoq>
nilay: Be able to use specify the stride factor is a nice addition, can you open a PR for the modification?
Mathnerd314 has joined #mlpack
< nilay>
zoq: ok sure I will open the PR for stride factor..
< nilay>
also, in the ForwardConvolutionRule we are preforming valid convolution not full convolution
< nilay>
if you look at conv_layer.hpp
< nilay>
so should we specify in such a case using template to perform FullConvolution
< nilay>
when we are padding...
< zoq>
Right, if you need FullConvolution in the forward pass just set ForwardConvolutionRule = NaiveConvolution<FullConvolution>.
< nilay>
but in the network, we don't specify anything when we call forward, how do we handle that case?
< nilay>
i mean when we call Forward function for the whole network
< zoq>
You have to instantiate the object with the correct parameter, e.g. ConvLayer<NaiveConvolution<ValidConvolution> > convLayer0(1, 8, 5, 5);
< zoq>
Can you also open a PR for the weight index correction?
< nilay>
should i open separate pr?
< zoq>
that would be great
< nilay>
zoq: did you test the Forward function in conv_layer.hpp, with testcase that I wrote above?
< nilay>
was i doing something wrong when using the current method?
< zoq>
No I didn't tested it with your test case, do you think you did something wrong?
< nilay>
actually after providing the padding i was using ValidConvolution because that is the default arguement
< nilay>
when we provide padding we have to use FullConvolution??
mentekid has joined #mlpack
< zoq>
Full convolution preserves the size, so depending on the input filter you need to pad the input.
< nilay>
actually what we want is same convolution (not full or valid). if we use ValidConvolution with padding also, then current implementation needs to be corrected. . .
< zoq>
Are you sure we need same convolution, the filter should also be smaller as the input, did I miss something?
< zoq>
Also, If you need same convolution you need to pad the input with half of the filter size.
< nilay>
that is what i am doing...?
< nilay>
zoq: if you could run the test that I wrote above, then you could see what I mean
< zoq>
okay, let's see
< zoq>
okay, I tested the code
< nilay>
zoq: thanks, did i do something wrong in calling functions in the test, why does the current implementation does not pass for me?
< zoq>
Not sure, do you get some error, or unexpected output?
< zoq>
I think you actually need same convolution, but we should implement it inside of the convolution function and not inside of the convolution layer. Since you don't want to do same convolution every time.
< nilay>
the test does not pass, right?
< zoq>
with your implementation I don't see an error
< nilay>
with the Forward method i modified, the test passes...
< nilay>
so we need to change it ?
< zoq>
Add same convolution to the convolution method, yes.
< nilay>
i mean with any padding, the current implementation will not give correct results
< nilay>
not only with same convolution
< zoq>
Wit the current implementation you mean your current or the current mlpack implementation?
< nilay>
if we give padding, the convOutput matrix, on line 91 in conv_layer.hpp will have lesser size than output.slice() matrix we are assigning it to on line 95
< nilay>
i mean current mlpack implementation in the master branch
< nilay>
because the input we get from the previous layer is of less size and we need to convolve with padded input.
< zoq>
I'm not sure I get your point, are we talking about same, full or valid convolution? Or are we talking about convolution in general?
< nilay>
i am talking about the Forward() method in conv_layer.hpp
< nilay>
if we give padding when making our conv_layer, and invoke that method, it'll give incorrect results..
< zoq>
I think I get what you mean, if we specify wPad and hPad the result is incorrect, right?
< nilay>
yes
< zoq>
right, because the output size dosn't match if wPad or hPad > 0
< nilay>
yeah
< zoq>
:)
< zoq>
okay, in this case you are absolutely right, looks like I never tested the code with wPad or hPad > 0
< nilay>
can you look at the function I modified, if it is the correct way to go about it?
< zoq>
Looks good to me, one thing we should do the padding if necessary inside the for loop in this case we don't have to copy the input if we don't have to pad the input.
< nilay>
ok thanks.
< zoq>
Thanks for pointing out the flaw in the code.
< zoq>
Once you opened the PR I'll go and write a test for the modification.
< zoq>
Also, you can combine the index and padding modification in a single PR.
< nilay>
I can write the test if you want, the test written above seems good? or we can modify it..
< zoq>
The test looks good, if you want you can include everything in your PR.
< nilay>
also, is my padding function good, you said something above, but it seems to give correct result here..
< zoq>
It's correct, I thought it would do something different.
< nilay>
ok
nilay has quit [Ping timeout: 250 seconds]
nilay has joined #mlpack
nilay has quit [Quit: Page closed]
nilay has joined #mlpack
sumedhghaisas has joined #mlpack
nilay has quit [Ping timeout: 250 seconds]
sumedhghaisas has quit [Ping timeout: 260 seconds]
mentekid has quit [Ping timeout: 258 seconds]
mentekid has joined #mlpack
travis-ci has joined #mlpack
< travis-ci>
mlpack/mlpack#1273 (master - ba850f7 : Ryan Curtin): The build was broken.