Skip to content

Fix early stopping in StopWhenRepeated#558

Merged
kellertuer merged 4 commits into
JuliaManifolds:masterfrom
JoshuaLampert:fix-stopwhenrepeated
Dec 10, 2025
Merged

Fix early stopping in StopWhenRepeated#558
kellertuer merged 4 commits into
JuliaManifolds:masterfrom
JoshuaLampert:fix-stopwhenrepeated

Conversation

@JoshuaLampert

Copy link
Copy Markdown
Contributor

I just had a case, where the interior stopping criterion of a StopWhenRepeated stopping criterion was satisfied at initialization, but not anymore in the first few iterations, i.e., it was not yet repeated n times, but the optimization was stopped nonetheless, which was confusing to me. This PR fixes this by not returning early in the initialization.

@codecov

codecov Bot commented Dec 10, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b4d1ada) to head (c7ab9b4).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #558   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           91        91           
  Lines         9976      9975    -1     
=========================================
- Hits          9976      9975    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kellertuer

Copy link
Copy Markdown
Member

That looks a bit strange to me now. Because now for k<=0 (we actually only call it for reset with -1) the criterion is called twice in 1339 and 1342. Maybe we should then remove 1339?

@JoshuaLampert

Copy link
Copy Markdown
Contributor Author

Ah yes, we call the criterion later anyway. You are right. Then it makes more sense to remove line 1339.

@kellertuer kellertuer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it looks better. Thanks for spotting that edge case.

Do you need that soon? Then I could release a new version in the evening.

@JoshuaLampert

Copy link
Copy Markdown
Contributor Author

I am currently using a development version of Manopt.jl anyway. So I don't really need it to be released today, but having it in a released version sooner than later would make it more comfortable. Thanks for asking.

@kellertuer

Copy link
Copy Markdown
Member

Sure no problem. If you want to help real quick, can you bump the version number in the project toml and set the date of the newest version in the Changelog to today? Then I do not have to do a second PR for that.

@JoshuaLampert

Copy link
Copy Markdown
Contributor Author

Sure.

@kellertuer
kellertuer merged commit 38cd7de into JuliaManifolds:master Dec 10, 2025
15 checks passed
@JoshuaLampert
JoshuaLampert deleted the fix-stopwhenrepeated branch December 10, 2025 17:16
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

Successfully merging this pull request may close these issues.

2 participants