@hweom sorry for the long delay, I see your point and I would appreciate a PR for this issue with a testcase. Happy to review. Practically this is not destructive, all values are summed in the following few lines, so some values get more weight and others are omitted.

Thanks for the answer. I'm still trying to wrap my head around the code. I think I understand why changing this part made no effect on the MNIST example -- since NLL layer is the last one, the output of it is not used anywhere.

Now I have a different question :) This definition of NLL that I found gives the formula:

y = -ln (x)

(here y is the output of the NLL layer and x is input for the correct class).

However, NegativeLogLikelihood::compute_output() does this:

y = -x

And similarly, the input gradient in NegativeLogLikelihood::compute_input_gradient() is computed like this:

for (batch_n, &label_value) in native_labels.iter().enumerate() {
    let index = (num_classes * batch_n) + label_value as usize;
    writable_gradient[index] = -1f32;
}

which is essentially ∂y/∂x = -1. This matches the way the output is computed, but doesn't quite match the definition of NLL.

FWIW, I tried changing compute_input_gradient() to compute the real NLL gradient with

writable_gradient[index] = -1.0 / native_probabilities[index].max(0.001);

(I didn't change compute_output() since it's not used, as mentioned above), but it actually caused training to stop converging.

Should the name of the layer be changed?

That is correct. The implementation is pretty old and was done by the original authors of the predecessor if juice.

The only explanation I have is when looking at the expansion of ln(x) which is

ln(x) ~= \sum_{n=0}^{\inf} - x + (1/2)x^2 - (1/3)x^3 + (1/4)x^4 + ..`

now if you terminate after the first term, you get the above. This saves a bunch of computations - ln is still rather slow, but at least two or 3 terms wouldn't hurt, but that would come at a quite significant additional cost of doing 3 more multiplications, a lock call, and an array lookup in the gradient compute.

I added a commit that would improve the approximation up to x^3 in #151

Is this a Taylor expansion of ln(x)? At which point? It doesn't look like one: example.

I'm probably missing something obvious...

https://math.stackexchange.com/questions/585154/taylor-series-for-logx says almost the above, an offset of -1 or respectively +1 is missing, do I guess that's another issue

You mean the most upvoted answer (https://math.stackexchange.com/a/585158)? Note that it has a formula for -ln(1-x), not ln(x).

That's what I meant in my previous comment

Well, if we take the Taylor expansion suggested by Wolfram:

and simplify it, we'll get this (https://www.wolframalpha.com/input/?i=expand+-%28%28x-1%29+-+1%2F2%28x-1%29%5E2+%2B+1%2F3%28x-1%29%5E3%29)

Or can just add the offset and keep the math cleaner.

I think the offset is not really relevant for learning.