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

Default ftol behavior #270

Open
yolking opened this issue Jan 18, 2025 · 1 comment
Open

Default ftol behavior #270

yolking opened this issue Jan 18, 2025 · 1 comment
Milestone

Comments

@yolking
Copy link

yolking commented Jan 18, 2025

Hello! Looking at, for example, diffev2 parameters documentation default ftol behavior can be very unexpected. By default I think most people would expect that it works by comparing cost function values of current and last generation.

ftol (float, default=5e-3) – acceptable relative error in cost(xopt) for convergence.

Looking though at code
https://github.com/uqfoundation/mystic/blob/0d79f7551543403e013b8a8136396cf853317359/mystic/differential_evolution.py#L803C1-L808C52
it's not actually how it works when gtol is None. In fact VTRChangeOverGeneration is called and among other things it compares current cost function value to target, which isn't even parameter of diffev2 function and equals 0 by default.

def VTRChangeOverGeneration(ftol=0.005, gtol=1e-6, generations=30, target=0.0):

In many cases user doesn't know predefined target he wants to achieve and it's not clear from documentation that this definition hides behind ftol parameter. In practice it means that optimization of some function taking both positive and negative values can randomly prematurely stop as soon as algorithm randomly stumbles in very close proximity to zero value of cost function what happened to me.
Image

I think this should be pointed out in documentation regarding algorithms' ftol parameter and not just in termination module regarding VTRChangeOverGeneration. Or maybe default behavior should be changed. Thank you for your attention

@mmckerns mmckerns added this to the mystic-0.4.4 milestone Jan 18, 2025
@mmckerns
Copy link
Member

Thanks for pointing that out. What I wanted was an easy way to change the termination condition (as one can do from the full class interface). Having target fixed at zero shouldn't be the case, and if the documentation isn't good enough, it shouldn't be an easter egg. I'll at the very least improve the docs.

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

No branches or pull requests

2 participants