[scala] Make accuracy idependant of output size (fix #8226)

[benqua]

Description

This PR change EvalMetric.sumMetric from Float to Double.

Fix #8226

[yzhliu]

Thanks @benqua , I think a better way is to change sumMetric to Double. The fix here changes the definition of Acc.

[benqua]

@Javelinjs , you're right, it changes the definition of accuracy for output.size > 1.
What is the exact definition of Accuracy? I couldn't find a clear definition.

This change provides a definition of accuracy that match the one from wikipedia for binary classification, that says:
the accuracy is the proportion of true results (both true positives and true negatives) among the total number of cases examined
(https://en.wikipedia.org/wiki/Accuracy_and_precision#In_binary_classification).

It seems weird (at least to me :) ) that the accuracy depends on the output dimension and can grow to very large numbers. By dividing by the label dimension, we keep the accuracy between 0 and 1, which is the expected range of a "proportion".

If we change sumMetric to Double, should we do it only for the value stored internally and keep float in the EvalMetric API?

[yzhliu]

We should keep it the same as other language bindings, especially python. What if we make it Double in EvalMetric API?

[benqua]

It can be done, but it would break the API for people calling EvalMetric.get (https://github.com/apache/incubator-mxnet/blob/master/scala-package/core/src/main/scala/ml/dmlc/mxnet/EvalMetric.scala#L52) .

[yzhliu]

We can convert it back to Float when calling EvalMetric.get

[benqua]

@Javelinjs ok, I updated the PR as you suggested.
(I should maybe have done a new one because the comment thread is now difficult to understand)
What do you think?

[benqua]

Ho, it seems there is a build issue. Not sure it is related to my code (something about the windows gpu build), but I will check this week-end.

[yzhliu]

LGTM.
The CI seems to have some problems these days. Lets try again this weekend.

[yzhliu]

@benqua could you rebase the pr and re-trigger the CI build?

[benqua]

Rebased. Waiting for the CI to complete.

[yzhliu]

@piiswrong Could you help to do a force merge?

[benqua]

I tried again, still no luck.
The message from the CI doesn't help to understand if it is still a CI issue or a PR one.

[yzhliu]

Sorry to see that. Please rebase again, I think the CI is OK now.

[benqua]

done. let's see...

[benqua]

It fails with:

FAIL: test_operator_gpu.test_svmoutput_with_type

----------------------------------------------------------------------

Traceback (most recent call last):

  File "C:\Anaconda3\envs\py2\lib\site-packages\nose\case.py", line 197, in runTest

    self.test(*self.arg)

  File "c:\jenkins_slave\workspace\ut-python-gpu\tests\python\gpu\test_operator_gpu.py", line 999, in test_svmoutput_with_type

    check_consistency(sym, ctx_list)

  File "c:\jenkins_slave\workspace\ut-python-gpu\pkg_vc14_gpu\python\mxnet\test_utils.py", line 1338, in check_consistency

    raise e

AssertionError: 

Items are not equal:

Error 5.000000 exceeds tolerance rtol=0.100000, atol=0.100000.  Location of maximum error:(18, 3), a=0.000000, b=1.000000

 a: array([[-1.,  1.,  1., ...,  1.,  1.,  1.],

       [-1.,  1.,  0., ...,  0.,  1.,  1.],

       [-1.,  1.,  1., ...,  1.,  1.,  1.],...

 b: array([[-1.,  1.,  1., ...,  1.,  1.,  1.],

       [-1.,  1.,  0., ...,  0.,  1.,  1.],

       [-1.,  1.,  1., ...,  1.,  1.,  1.],...

-------------------- >> begin captured stdout << ---------------------

Train Err: ctx 2 vs ctx 0 at svmoutput_data


--------------------- >> end captured stdout << ----------------------


----------------------------------------------------------------------

Ran 243 tests in 1392.034s


FAILED (SKIP=5, failures=1)


(py2) c:\jenkins_slave\workspace\ut-python-gpu>IF 1 NEQ 0 exit /b 1 

script returned exit code 1

will investigate when I find time. idea, help welcome :)

[marcoabreu]

I've encountered the same error with test_operator_gpu.test_svmoutput_with_type on my setup based on the release brach v0.12.0. Please check our internal wiki, @KellenSunderland @mbaijal @larroy

[marcoabreu]

https://builds.apache.org/blue/organizations/jenkins/incubator-mxnet/detail/PR-8297/13/pipeline

line 452

[ERROR] /workspace/scala-package/core/src/main/scala/ml/dmlc/mxnet/EvalMetric.scala:115: error: not found: value la
[INFO] this.sumMetric += la.zip(predLabel.toArray)

That’s clearly not a CI issue

[benqua]

@Javelinjs , Finally, the PR passes all tests.
Do you think it can be merged?

[yzhliu]

Thanks.