search

scalanlp / breeze

Breeze is/was a numerical processing library for Scala.
https://www.scalanlp.org
Apache License 2.0
3.45k stars 693 forks source link

FirstOrderMinimizer#defaultConvergenceCheck fails with negative function values #764

Closed Antti-Paladin closed 4 years ago

Antti-Paladin commented 4 years ago

I noticed that the default convergence check for FirstOrderMinimizer does not use absolute value of the function, when doing a check for relative gradient. I think when minimizing functions that yield negative values at the current considered point, the tolerance for gradient norm always falls back to 1E-8.

Fixed to use the absolute value of the optimized function. Could you please check if this could be merged?

EDIT: Seems the CI pipeline doesn't pass, but the tests passed locally for me.

Antti-Paladin commented 4 years ago

Has someone taken a look at this? I think it's very similar to https://github.com/scalanlp/breeze/issues/664, just this happens when checking the gradient.