Skip to content

Cast max_stepsize in Mesh Adaptive Direct Seach (#526)#527

Merged
kellertuer merged 5 commits into
JuliaManifolds:masterfrom
sblelong:MADS-injectivity-radius-fix
Oct 31, 2025
Merged

Cast max_stepsize in Mesh Adaptive Direct Seach (#526)#527
kellertuer merged 5 commits into
JuliaManifolds:masterfrom
sblelong:MADS-injectivity-radius-fix

Conversation

@sblelong

Copy link
Copy Markdown
Contributor
  • Explicit cast of MADS' maximum stepsize to match the mesh scaling parameter type
  • Unit test of MADS running on a sphere

@codecov

codecov Bot commented Oct 28, 2025

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #527   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          86       86           
  Lines        9781     9781           
=======================================
  Hits         9760     9760           
  Misses         21       21           

☔ 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 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.

Thanks, I did not know the function oftype! I have just one comment on the code PR itself.

For the failing tests

Comment thread test/solvers/test_mesh_adaptive_direct_search.jl Outdated
@sblelong

Copy link
Copy Markdown
Contributor Author

Thanks a lot for your quick feedback! I was actually trying to figure out how to reach out to you on Slack or Zulip since the failing tests led to some panicking on my side... It is my first PR on a fairly large repo so, thank you for bearing with me. :)

I'm working on fixing those two errors.

I may have missed something, but I think the contributing guidelines ask to run JuliaFormatter.jl instead of Runic.jl. I guess the switch to runic was quite recent, maybe not yet reverberated to the guidelines?

@kellertuer

Copy link
Copy Markdown
Member

Oh, did I miss to update the guidelines, sorry for that! Will check that tomorrow – and fix that if necessary;
I also am already on mobile and have turned of slack and Zulip (in CET its 9:30pm).

I'm sure you'll manage, no reason to panic - just keep calm and code on :)

@sblelong

Copy link
Copy Markdown
Contributor Author

There's absolutely no harm in that, the fact that all files of the repo were modified when applying JuliaFormatter should've been a hint for me that another formatter was in use...

Thanks all the more so that it's getting late! Hope those failing tests weren't too much of a disturbance in the evening. I'll take advantage of the remaining sun time I have to fix those issues.

@kellertuer kellertuer mentioned this pull request Oct 28, 2025
@kellertuer

Copy link
Copy Markdown
Member

Oh usually the PR fixing an issue would close the issue, now you closed it a bit early, but never mind :) (if you mention in the first comment or somewhen along that this PR closes/fixes #anothernumber then that automatically closes it)

@sblelong

Copy link
Copy Markdown
Contributor Author

Oops... Sorry again. I feared keeping it open would be confusing since the issue is somehow addressed. But that's one more good practice I'll keep in mind for the future.

@kellertuer

Copy link
Copy Markdown
Member

It is not wrong, just a bit unusual, for example if this would only fix something half ;)

@kellertuer

Copy link
Copy Markdown
Member

I did adapt my proposal of the tests above and the tests now also directly use Manifolds.jl 0.11.2, but we could enforce that a bit better by changing the compat entry in Project.toml as well.

@sblelong

Copy link
Copy Markdown
Contributor Author

Indeed, using Manifolds.jl 0.11.2 solves that. The new tests seem to pass perfectly.

@kellertuer

Copy link
Copy Markdown
Member

Thanks again. I will include this in the next release :)

@kellertuer
kellertuer merged commit 6edc099 into JuliaManifolds:master Oct 31, 2025
18 of 20 checks passed
@sblelong

Copy link
Copy Markdown
Contributor Author

My pleasure :)

kellertuer referenced this pull request Nov 5, 2025
* Introduce a vector bundle Newton method
* documentation
* Code coverage.

---------

Co-authored-by: Laura Weigl <Laura.Weigl@uni-bayreuth.de>
Co-authored-by: Anton Schiela <anton.schiela@uni-bayreuth.de>
Co-authored-by: Hajg Jasa <hajg.jasa@ntnu.no>
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