Open ChristophHornung opened 2 years ago
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?
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?
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.
@eriove I opened a PR (#951) if you want you can check the convergence in your use case to see if it improved.
@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).
@eriove Ok, thanks for checking.
Is there anything further I should do to get the PR approved?
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 hereNamely
TryToScaleSimplex
functions recalculate the centroid instead of calculating it only once per iterationIs there any reason for those changes or a source paper? Or are those bugs?