NaN should map to -Inf for maximizer (#71)

* nan for max should map to neg inf * add tests for nan handling * fix test (and workaround single param bug) * Fix tests... * should run tests locally first...
baggepinnen · Feb 10, 2022 · bf901f3 · bf901f3
1 parent e207cec
commit bf901f3
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 2 deletions.
diff --git a/src/Hyperopt.jl b/src/Hyperopt.jl
@@ -40,8 +40,8 @@ end
 function Base.getproperty(ho::Hyperoptimizer, s::Symbol)
     s === :minimum && (return isempty(ho.results) ? NaN :  minimum(replace(ho.results, NaN => Inf)))
     s === :minimizer && (return isempty(ho.results) ? [] :  ho.history[argmin(replace(ho.results, NaN => Inf))])
-    s === :maximum && (return isempty(ho.results) ? NaN :  maximum(replace(ho.results, NaN => Inf)))
-    s === :maximizer && (return isempty(ho.results) ? [] :  ho.history[argmax(replace(ho.results, NaN => Inf))])
+    s === :maximum && (return isempty(ho.results) ? NaN :  maximum(replace(ho.results, NaN => -Inf)))
+    s === :maximizer && (return isempty(ho.results) ? [] :  ho.history[argmax(replace(ho.results, NaN => -Inf))])
     return getfield(ho,s)
 end
 

diff --git a/test/runtests.jl b/test/runtests.jl
@@ -41,6 +41,14 @@ end
         @test length(hor.history) == 102
         @test length(hor.results) == 102
 
+        # Test NaN handling
+        ho2 = @hyperopt for i=2, sampler=RandomSampler(), a = [20], b = [1]
+            i == 1 ? a*b : NaN
+        end
+        @test ho2.minimum == 20
+        @test ho2.maximum == 20
+        @test ho2.minimizer == [20, 1] 
+        @test ho2.maximizer == [20, 1]
     end
 
     @testset "Latin hypercube" begin