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

do not allocate xtol_abs unless needed #268

Merged
merged 9 commits into from
Nov 21, 2020

Conversation

aitap
Copy link
Contributor

@aitap aitap commented Apr 26, 2019

As discussed in #183, it is beneficial to avoid allocating potentially
huge buffers of size n unless x-tolerance criteria are used.

This patch adds checks for !opt->xtol_abs and behaves as if
xtol_abs[i] == 0 ∀i if true.

@stevengj
Copy link
Owner

There are also a few other explicit references to xtol_abs that need to be fixed: in bobyqa.c, in cdirect.c, in hybrid.c, in cobyla.c, in sbplx.c, in newuoa.c, in praxis.c, in stogo/local.cc, in api/optimize.c

@aitap
Copy link
Contributor Author

aitap commented Apr 26, 2019

Of course, I should have read the code instead of relying on make test to produce memory errors.

There are parts in cdirect.c and hybrid.c where both stopping criteria on x seem to be checked manually, except the relative stopping criterion is

dx[i] <= (ub[i] - lb[i]) * xtol_rel // ∀ i
// where lb and ub are 0 and 1 unless NOSCAL is used

instead of dx[i] < xtol_rel * x[i] // ∀ i or ||∆x|| < xtol_rel * ||wx||.

If this is integral to the algorithm, I can preserve the current behaviour by adding checks:

-	  if (w[i] > (p->stop->xtol_abs[i] ) &&
+	  if (w[i] > (p->stop->xtol_abs ? p->stop->xtol_abs[i] : 0) &&
- && w[i] > p->stop->xtol_abs[i])
+ && w[i] > p->stop->xtol_abs ? p->stop->xtol_abs[i] : 0)

Is this the right thing to do, though?

@stevengj
Copy link
Owner

@aitap, I think that code predates the code in stop.c — it can be replaced with calls to stop.c routines

@aitap
Copy link
Contributor Author

aitap commented Apr 27, 2019

Sorry, my analysis was wrong. Those are not dx, but widths of hyper-rectangle sides. The (ub[i] - lb[i]) * xtol_rel criterion makes much more sense now.

I tried to avoid branching in for-loops, but in some cases letting a ternary operator in led to more comprehensible code.

@stevengj stevengj merged commit bc5f39e into stevengj:master Nov 21, 2020
@stevengj
Copy link
Owner

LGTM, thanks!

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