[Repo Assist] fix: remove CheckFunctionEpsilon from NelderMead stop criteria#377
Draft
github-actions[bot] wants to merge 2 commits intodeveloperfrom
Conversation
…riteria The IsCriteria function in OptimizationStop.fs included two calls to CheckFunctionEpsilon which stops optimization when the function value drops below MinFunctionEpsilon (default 1e-8). This caused premature termination for any function with a minimum near or below zero — including all functions with negative optima. Remove the two CheckFunctionEpsilon calls from IsCriteria. The check is appropriate for constrained positive-only optimisation methods but is incorrect for Nelder-Mead, which handles arbitrary real-valued functions. The stationary-point and iteration-limit checks correctly handle convergence. Update PSF test to compare against the true minimum (0,0,0,0) since the optimizer now converges correctly. Add a new test for the quadratic function from issue #260 that has a negative minimum at x=0.16, f(x)≈-0.1556. Closes #260 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## developer #377 +/- ##
=============================================
+ Coverage 47.61% 47.87% +0.25%
=============================================
Files 152 135 -17
Lines 16765 12920 -3845
Branches 2253 1709 -544
=============================================
- Hits 7983 6185 -1798
+ Misses 8097 6221 -1876
+ Partials 685 514 -171 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
25 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This is an automated pull request from Repo Assist, an AI assistant.
Summary
Fixes the premature termination bug in
NelderMead.minimizewhen the objective function has negative or near-zero values.Root cause:
OptimizationStop.IsCriteriacalledCheckFunctionEpsilonon bothfoldandfnew. This check stops optimization whenever a function value is belowMinFunctionEpsilon(default1e-8). Since negative values are always below1e-8, any function with a negative minimum caused early, incorrect termination.Changes
src/FSharp.Stats/Optimization/OptimizationStop.fsCheckFunctionEpsiloncalls fromIsCriteria.The check is appropriate for constrained positive-only methods but is wrong for Nelder-Mead, which is a derivative-free method for general real-valued functions.
Correct convergence is already handled by
CheckStationaryPointandCheckIteration.tests/FSharp.Stats.Tests/Optimization.fs(0,0,0,0)instead of the previously hardcoded "stopped-too-early" values. The fixed optimizer now converges tox ≈ (3.8e-5, -3.8e-6, -2.8e-5, -2.8e-5)— much closer to the true optimum.f(x) = x2 - 0.32x - 0.13, minimum atx=0.16,f(x*)≈-0.1556) that directly reproduces the bug from issue [Feature Request] Documentation of Nelder-Mead method #260. This test failed before the fix.Test Status
All 1195 tests pass, including 8 NelderMead tests (2 new).
Closes #260