Repository: JuliaAI/MLJBase.jl (julia)

Issue:
Handling of NaN micro/macro aggregation
I think it might be worth reviewing the current handling of `NaN` in multiclass micro/macro aggregation and in binary `FScore`. 
At present a method called `_mean` substitutes zero for any `NaN` in the one-against-many fscores, before aggregation. Perhaps the `NaN`' should just be skipped?? Also, it appears the `NaN` will arise whenever the precision or recall is zero. However, even in the binary case, FScore could be well-defined even if both precision and recall are zero:

precision = TP/(TP +FP)
recall = TP/(TP + FN)

score is usually defined as harmonic mean of these (and so undefined if either vanishes); however, if you simplify the harmonic mean expression (elminate a common factor of TP from final denominator and numerator) you get:

score = 2TP/(2TP + FP + FN)

which is only undefined if every sample is TN.  

cc @ven-k 

**added after discussion below**

To do: Replace harmonic mean of precision and recall with explicit reduced formula involving basic counts, as indicated above.

- [x] MulticlassFScore
- [x] Vanilla binary FScore
- [ ] Change substitution with 0 with skip; see [comment below](https://github.com/JuliaAI/MLJBase.jl/issues/630#issuecomment-922236280)

Pull request description:
Resolves #630

Patch:
diff --git a/src/measures/finite.jl b/src/measures/finite.jl
index 385838a3..62dff7c0 100644
--- a/src/measures/finite.jl
+++ b/src/measures/finite.jl
@@ -966,12 +966,7 @@ function _mtn(m::CM, return_type::Type{LittleDict})
     return LittleDict(m.labels, vec(_sum))
 end
 
-@inline function _mean(x::Arr{<:Real})
-    for i in eachindex(x)
-        @inbounds x[i] = ifelse(isnan(x[i]), zero(eltype(x)), x[i])
-    end
-    return mean(x)
-end
+@inline _mean(x::Arr{<:Real}) = mean(skipnan(x)) # defined in src/data/data.jl
 
 @inline function _class_w(level_m::Arr{<:String},
                           class_w::AbstractDict{<:Any, <:Real})


Test patch:
diff --git a/test/measures/finite.jl b/test/measures/finite.jl
index edd3903b..f06266c3 100644
--- a/test/measures/finite.jl
+++ b/test/measures/finite.jl
@@ -284,6 +284,28 @@ end
     2 ./ ( 1 ./ ( sum(cm_tp) ./ sum(cm_fp.+cm_tp) ) + 1 ./ ( sum(cm_tp) ./ sum(cm_fn.+cm_tp) ) )
 end
 
+@testset "issue #630" begin
+    # multiclass fscore corner case of absent class
+
+    y = coerce([1, 2, 2, 2, 3], OrderedFactor)[1:4]
+    # [1, 2, 2, 2] # but 3 is in the pool
+    yhat = reverse(y)
+    # [2, 2, 2, 1]
+
+    # In this case, assigning "3" as "positive" gives all true negative,
+    # and so NaN for that class's contribution to the average F1Score,
+    # which should accordingly be skipped.
+
+    # postive class | TP | FP | FN | score for that class
+    # --------------|----|----|----|---------------------
+    #  1            | 0  | 1  | 2  | 0
+    #  2            | 2  | 1  | 1  | 2/3
+    #  3            | 0  | 0  | 0  | NaN
+
+    # mean score with skippin NaN is 1/3
+    @test MulticlassFScore()(yhat, y) ≈ 1/3
+end
+
 @testset "Metadata binary" begin
     for m in (accuracy, recall, Precision(), f1score, specificity)
         e = info(m)
diff --git a/test/resampling.jl b/test/resampling.jl
index e392fbbe..997f9521 100644
--- a/test/resampling.jl
+++ b/test/resampling.jl
@@ -486,24 +486,43 @@ end
 end
 
 @testset_accelerated "class weights in evaluation" accel begin
-    x = [1,2,3,4,5,6,7]
-    X, y = Tables.table([x x x x x x]), coerce([1,2,1,3,1,2,2], Multiclass)
-    model = Models.DeterministicConstantClassifier()
-    mach = machine(model, X, y)
+    X, y = make_blobs(rng=rng)
     cv=CV(nfolds = 2)
+    fold1, fold2 = partition(eachindex(y), 0.5)
+    m = MLJBase.MulticlassFScore()
     class_w = Dict(1=>1, 2=>2, 3=>3)
-    e = evaluate!(mach,
-                  resampling=cv,
-                  measure=MLJBase.MulticlassFScore(return_type=Vector),
-                  class_weights=class_w,
-                  verbosity=verb,
-                  acceleration=accel).measurement[1]
-    @test round(e, digits=3) ≈ 0.217
+
+    model = Models.DeterministicConstantClassifier()
+    mach = machine(model, X, y)
+
+    # fscore by hand:
+    fit!(mach, rows=fold1, verbosity=0)
+    score1 = m(predict(mach, rows=fold2), y[fold2], class_w)
+    fit!(mach, rows=fold2, verbosity=0)
+    score2 = m(predict(mach, rows=fold1), y[fold1], class_w)
+    score_by_hand = mean([score1, score2])
+
+    # fscore by evaluate!:
+    score = evaluate!(
+        mach,
+        resampling=cv,
+        measure=m,
+        class_weights=class_w,
+        verbosity=verb,
+        acceleration=accel,
+    ).measurement[1]
+
+    @test score ≈ score_by_hand
 
     # if class weights in `evaluate!` isn't specified:
-    e = evaluate!(mach, resampling=cv, measure=multiclass_f1score,
-                  verbosity=verb, acceleration=accel).measurement[1]
-    @test e ≈ 0.15
+    plain_score = evaluate!(
+        mach,
+        resampling=cv,
+        measure=m,
+        verbosity=verb,
+        acceleration=accel,
+    ).measurement[1]
+    @test !(score ≈ plain_score)
 end
 
 @testset_accelerated "resampler as machine" accel begin
@@ -794,4 +813,3 @@ end
 
 #end
 true
-