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

Uncommon Nelder-Mead algorithm (Bugs?) #950

Open
ChristophHornung opened this issue Sep 6, 2022 · 6 comments
Open

Uncommon Nelder-Mead algorithm (Bugs?) #950

ChristophHornung opened this issue Sep 6, 2022 · 6 comments

Comments

@ChristophHornung
Copy link

ChristophHornung commented Sep 6, 2022

I was checking the implementation of the NedlerMeadSimplex algorithm while looking at a few different code-implementations in order to better understand the algorithm and noticed a few discrepancies with how the algorithm is described both here and here

Namely

  • The expansion step uses a chi-value of -2 (2 as the function parameter but the function actually flips the sign in comparison with the paper)
  • All TryToScaleSimplex functions recalculate the centroid instead of calculating it only once per iteration

Is there any reason for those changes or a source paper? Or are those bugs?

@eriove
Copy link
Contributor

eriove commented Sep 6, 2022

I did part of the implementation (mainly refactoring and merging finding common interfaces) and have not gotten it to work properly for real world cases, it works for he test though. Hence I would suspect that you've found a bug.

Are the tests still passing if you change these things?

@ChristophHornung
Copy link
Author

I haven't changed the code yet, just mainly tried to understand why the algorithm is different (it also seems to only do the 'inside contraction' step and omit the 'outside contraction'.). I wondered if it may be adapted from a different source then the ones I am seeing. After all there are a few papers on how to improve NelderMead.
I can change the algorithm to how it is described in the references I gave to see what happens to the tests.

Do you have a real world case that fails at the moment to see if the changes improve the outcome?

@eriove
Copy link
Contributor

eriove commented Sep 6, 2022

I honestly can't remember. I don't think I was the one that implemented it, but it has been too long for me to remember.

I have a case that fails (does not converge) that I can test to see if the outcome is improved. Unfortunately I can't share the the code but if you make changes I can try them.

@ChristophHornung
Copy link
Author

@eriove I opened a PR (#951) if you want you can check the convergence in your use case to see if it improved.

@eriove
Copy link
Contributor

eriove commented Sep 13, 2022

@eriove I opened a PR (#951) if you want you can check the convergence in your use case to see if it improved.

Unfortunately it did not make a difference. I suspect it is due to local steps in my target functions. This probably doesn't say anything about your implementation, just my annoying target functions.

I would vote for switching to your implementation in MathNet Numerics, since it more closely follows a paper and is easier to maintain (if needed).

@ChristophHornung
Copy link
Author

@eriove Ok, thanks for checking.

Is there anything further I should do to get the PR approved?

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