Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-zero return value of a callback function is used as a return value of lbfgs() #36

Open
msakai opened this issue Jun 22, 2023 · 0 comments · May be fixed by #37 or #38
Open

Non-zero return value of a callback function is used as a return value of lbfgs() #36

msakai opened this issue Jun 22, 2023 · 0 comments · May be fixed by #37 or #38

Comments

@msakai
Copy link
Contributor

msakai commented Jun 22, 2023

I noticed that when a callback function returns non-zero value, the value is used as a return value of lbfgs().

liblbfgs/lib/lbfgs.c

Lines 495 to 500 in 5ad02fb

/* Report the progress. */
if (cd.proc_progress) {
if ((ret = cd.proc_progress(cd.instance, x, g, fx, xnorm, gnorm, step, cd.n, k, ls))) {
goto lbfgs_exit;
}
}

But this behavior is not documented, e.g. at

liblbfgs/include/lbfgs.h

Lines 403 to 404 in 5ad02fb

* @retval int Zero to continue the optimization process. Returning a
* non-zero value will cancel the optimization process.

or

liblbfgs/include/lbfgs.h

Lines 461 to 465 in 5ad02fb

* @param proc_progress The callback function to receive the progress
* (the number of iterations, the current value of
* the objective function) of the minimization
* process. This argument can be set to \c NULL if
* a progress report is unnecessary.

The current situation is not desirable as a user might write a callback function that returns a value that is not intended to be used as the return value of lbfgs().

I think it's better to either:

  1. document the current behavior (Document that non-zero return value of a callback is used as return value of lbfgs() #37), or
  2. make lbfgs() always return LBFGSERR_CANCELED on cancel, ignoring the actual return value of the callback (Make lbfgs() to return LBFGSERR_CANCELED when callback function returns non-zero value #38)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant