Added test for Cross Entropy derivative #80
Conversation
|
Hi! Yes the crossentropy one is a bit of a loner. It is a bit of a hack and doesn't quite fit in the overall design, yet it seems useful to have. |
|
@Evizero is the current implementation of |
|
Looks good to me! does it pass the tests? |
|
It passes locally on my laptop, will have to wait for travis checks. |
|
|
||
| immutable CrossentropyLoss <: SupervisedLoss end | ||
| typealias LogitProbLoss CrossentropyLoss | ||
|
|
||
| function value(loss::CrossentropyLoss, target::Number, output::Number) | ||
| target >= 0 && target <=1 || error("target must be in [0,1]") | ||
| output >= 0 && output <=1 || error("output must be in [0,1]") |
There was a problem hiding this comment.
I am unsure if we should do such checks. we don't usually do them because of the nature of these functions (they get called very often in some inner loop). So normally we assumer the caller knows what he/she is doing.
I am ok with this approach as well though.
There was a problem hiding this comment.
I would vote to remove the checks personally. But I don't feel strongly enough to hold up a merge
There was a problem hiding this comment.
@Evizero @tbreloff
Agreed with your concerns, but I think checks are necessary (espescially for libraries which other libraries are going to use). I would prefer an error rather than a strange behaviour in my code.
There are checks in PeriodicLoss, HuberLoss and epsilon losses.
Though I am not much experienced but in SymPy they have used checks for every possible corner case. Also Julia is awesome in showing errors:
ERROR: output must be in [0,1]
in value(::LossFunctions.CrossentropyLoss, ::Int64, ::Int64) at /home/nishnik/.julia/v0.5/LossFunctions/src/supervised/other.jl:44
Sorry, I am not much experienced in Julia to strongly pitch this, it is just my thinking. If you want me to remove checks I would do it, but I am a little reluctant to do this.
Otherwise ready to merge from my side.
There was a problem hiding this comment.
There are checks in PeriodicLoss, HuberLoss and epsilon losses.
The difference there is that the check is in the constructor, while here it is in the computational kernel. The later is executed much more often in real use.
Let's keep it for now
|
Soft ping @Evizero ! |
|
|
||
| immutable CrossentropyLoss <: SupervisedLoss end | ||
| typealias LogitProbLoss CrossentropyLoss | ||
|
|
||
| function value(loss::CrossentropyLoss, target::Number, output::Number) | ||
| target >= 0 && target <=1 || error("target must be in [0,1]") | ||
| output >= 0 && output <=1 || error("output must be in [0,1]") |
There was a problem hiding this comment.
I would vote to remove the checks personally. But I don't feel strongly enough to hold up a merge
|
Thanks for working on this |
While looking in coveralls, I found that
derivforCrossentropyis not called even once.Added proper documentation for loss functions in
other.jlIf fine, I would extend it to
deriv2.