Skip to content
New issue

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

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

Already on GitHub? # 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.

@aitap aitap force-pushed the xtol_abs_noalloc branch from abc053d to ceed328 Compare April 27, 2019 14:09
@stevengj stevengj merged commit bc5f39e into stevengj:master Nov 21, 2020
@stevengj
Copy link
Owner

LGTM, thanks!

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

Successfully merging this pull request may close these issues.

2 participants