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

[ENH] Raising exceptions instead of printing warnings for errors. #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wolfgang-noichl
Copy link

Currently, it is not possible to react on LBFGS errors in a meaningful way:

Quite some errors (eg LBFGSERR_MAXIMUMITERATION) wouldn't render the result completely useless - one might be still interested in x. However, since they trigger an exception, x is not accessible anymore. Just issueing a warning for these doesn't seem a good idea either, since a warning cannot be dealt with in an automatic way.

This is a vague proposal on how to do this differently:

  • all warnings are replaced by (different!) exceptions
  • because these would prevent returning x, x can (optionally) be written back to a parameter.

There might be a more clean way to do this, I'm happy for suggestions

      For this to make sense, the result is written back into an
      (optional) parameter instead of being returned directly.
@fgregg
Copy link

fgregg commented Apr 7, 2016

Actually, sorry for my previous comment. I thought I understood what you were intending, but I think I do not.

If an "error" message raised by the C code is recoverable, it seems like we should show that as a warning. I think we should just add those "recoverable" conditions to this line: https://github.com/datamade/pylbfgs/blob/master/lbfgs/_lowlevel.pyx#L397

@wolfgang-noichl
Copy link
Author

Yeah, that would help already.

There's at least LBFGSERR_ROUNDING_ERROR, LBFGSERR_MAXIMUMLINESEARCH, LBFGSERR_MAXIMUMITERATION (maybe even LBFGSERR_INCREASEGRADIENT), that might, depending on the actual application, be recoverable.

So my suggestion is raising an exception on any of these, and then let the user decide (with try/catch) how to deal with them.

E.g., one might decide that LBFGSERR_MAXIMUMLINESEARCH can be recovered from, whereas other errors cannot. Then, with my patch you could do:

try:
    lbfgs.minimize(..., x_result)
except errors.MaximumLinesearch:
    pass

# result can be obtained from x_result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants