Fix binary crossentropy double epsilon

[fhennecker]

When I compute a binary crossentropy with Keras betweeen a target value == 1 and a prediction value == 0, the crossentropy formula contains a log(0) which does not exist and is avoided adding a small epsilon value.

The epsilon has always been 1e-7 so far, and so the numerical value of the binary crossentropy in the case described above is supposed to be equal to -log(1e-7) == 16.11809539794922

In previous versions of Keras this was true. However, on (at least) tensorflow 2.2, it's equal to 15.424948470398375 which is the same as 2e-7. This is because the binary crossentropy code:

1. clips the predictions between (epsilon, 1-epsilon)
2. adds an epsilon when computing the log

This results in a clipping between (2*epsilon, 1) which I believe is not the expected behaviour.

This pull request removes the second step.

Here is a snippet to reproduce:

from tensorflow.keras.losses import BinaryCrossentropy
from tensorflow.keras import backend
import numpy as np

loss = BinaryCrossentropy()
print(backend.eval(loss(np.array([1.]), np.array([0.]))))

If this is indeed a bug, I can also open an issue for easier traceability.

[pavithrasv]

Thank you for the PR!

[gbaned]

Here are the internal errors, @fhennecker can you please verify ?

Traceback (most recent call last):
File "/unittest/case.py", line 59, in testPartExecutor
yield
File "/unittest/case.py", line 605, in run
testMethod()
File "/third_party/py/absl/testing/parameterized.py", line 282, in bound_param_test
return test_method(self, **testcase_params)
File "//tensorflow/python/framework/test_combinations.py", line 315, in decorated
execute_test_method()
File "//tensorflow/python/framework/test_combinations.py", line 298, in execute_test_method
test_method(**kwargs_to_pass)
File "//tensorflow/python/keras/losses_test.py", line 766, in test_sample_weighted
self.assertAlmostEqual(self.evaluate(loss), 4.6, 3)
File "/unittest/case.py", line 878, in assertAlmostEqual
raise self.failureException(msg)
AssertionError: 4.782716 != 4.6 within 3 places

[gbaned]

@fhennecker, Any update on this PR? Please. Thanks!

[fhennecker]

@pavithrasv @gbaned sorry was away for a couple of weeks.

I changed the expected values for all binary crossentropy tests that were affected and updated explanatory comments. They were pretty explicitly "using" double epsilons, I hope the behaviour I'm proposing is still the expected one.

[pavithrasv]

Thank you @fhennecker . The change looks correct to me, however since this could be a big breaking change let me run a few tests to see how much impact this has.

[pavithrasv]

As suspected this will be a breaking change breaking a lot of other Google users. I will need to make all of these changes together. Will make sure to keep this PR updated.

[gbaned]

@pavithrasv Any update on this PR? Please. Thanks!

[pavithrasv]

@gbaned Sorry about the delay, i have been busy working on other items. I will try to make this change sometime within the next couple of weeks.

[fhennecker]

Will try to look at it today, or next week.

[fhennecker]

@gbaned @fchollet Should be fixed now -- I also rebased on master

[fchollet]

There are ongoing test failures -- please fix.

[fhennecker]

I fixed what I could fix, but if the tests fail again I might need a bit of help. I couldn't access the details of the failures for some of the github checks, and I ran into quite a lot of trouble trying to run the tests locally, but at least the Ubuntu CPU check should be fixed.

[fchollet]

Let's try this again.

[fhennecker]

Ok, so it looks like there are 3 last failing steps: import/copybara, and both Windows Bazel builds. I have tried looking into why they're failing but have no idea how to fix them (and I don't have a Windows machine at hand). Do you guys have any pointers for this?

[fchollet]

The PR says "merged", but in practice it isn't merged. What happened?