Weird Condition in SGD

[aytekinar]

In this file, there is a condition on line 63 which says:

if (overallObjective != overallObjective)
{
    Log::Warn << "SGD: converged to " << overallObjective << "; terminating"
        << " with failure.  Try a smaller step size?" << std::endl;
    return overallObjective;
}

Is this a bug, or what is the reasoning behind it?

[rcurtin]

Hi there,

If the step size is too large, the optimization may jump back and forth in a valley, taking progressively larger steps, until the overall objective diverges to NaN. That's the justification for the warning. In various situations where I've used SGD (that led to this comment being added), reducing the step size was usually a solution to the problem that caused convergence to a reasonable objective value.

I hope this is helpful; let me know if I should clarify further.

Thanks,
Ryan

[aytekinar]

Hey,

Thanks for the answer. Sorry --- apparently I had not made myself clear enough. I was trying to ask the overallObjective != overallObjective comparison, actually. Do you say putting this comparison is good because when the objective function is inf (or, is it really nan?), it will not be equal to itself?

Thanks for your time

[rcurtin]

Yep, that's exactly it. See here:
http://stackoverflow.com/questions/570669/checking-if-a-double-or-float-is-nan-in-c
(this actually suggests that the way I've written it isn't the best way, so perhaps it should be changed at some point...)

[stereomatchingkiss]

Hi, maybe you can consider using

 bool isnan( double arg );

in c++11(any intention to upgrade to c++14?), maybe this is more portable and easier to read

[aytekinar]

Yes, I agree with @stereomatchingkiss . My intention was not to disturb you with the subtleties like this, but to understand what was going on. Yet, if your style guide is not strictly against the use of C++11 or above, you might consider using bool std::isnan( double arg ) or bool std::isinf(double arg), both from the #include <cmath> library. That would make the code easier to read and more robust against compiler-dependent implementation issues. Thanks for the discussion!

[rcurtin]

Fixed in 09cd0d6; thanks for the suggestions. I think it's important to consider little readability details like this, so if you find anything else you think should be changed, please feel free to open more issues. :)