Skip to content

Remove kwargs for options#1186

Merged
pkofod merged 23 commits intomasterfrom
pkm/no_kwargs
Nov 1, 2025
Merged

Remove kwargs for options#1186
pkofod merged 23 commits intomasterfrom
pkm/no_kwargs

Conversation

@pkofod
Copy link
Member

@pkofod pkofod commented Oct 31, 2025

I may re-instate inplace and autodiff for the function input api

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025

Benchmark Results

master c490158... master / c490158...
multivariate/solvers/first_order/AdaMax 0.636 ± 0.0079 ms 0.632 ± 0.0079 ms 1.01 ± 0.018
multivariate/solvers/first_order/Adam 0.636 ± 0.0081 ms 0.634 ± 0.008 ms 1 ± 0.018
multivariate/solvers/first_order/BFGS 0.221 ± 0.0044 ms 0.219 ± 0.0038 ms 1.01 ± 0.027
multivariate/solvers/first_order/ConjugateGradient 0.049 ± 0.00064 ms 0.0486 ± 0.00057 ms 1.01 ± 0.018
multivariate/solvers/first_order/GradientDescent 1.66 ± 0.01 ms 1.66 ± 0.0097 ms 1 ± 0.0084
multivariate/solvers/first_order/LBFGS 0.217 ± 0.0042 ms 0.214 ± 0.0037 ms 1.01 ± 0.026
multivariate/solvers/first_order/MomentumGradientDescent 2.44 ± 0.015 ms 2.43 ± 0.011 ms 1 ± 0.0077
multivariate/solvers/first_order/NGMRES 0.54 ± 0.011 ms 0.536 ± 0.011 ms 1.01 ± 0.029
time_to_load 0.462 ± 0.0043 s 0.458 ± 0.0046 s 1.01 ± 0.014

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

❌ Patch coverage is 65.00000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.79%. Comparing base (07faeee) to head (c490158).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/multivariate/solvers/constrained/fminbox.jl 36.36% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1186      +/-   ##
==========================================
+ Coverage   85.73%   85.79%   +0.06%     
==========================================
  Files          46       45       -1     
  Lines        3604     3577      -27     
==========================================
- Hits         3090     3069      -21     
+ Misses        514      508       -6     

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

F::Fminbox = Fminbox(),
options::Options = Options();
inplace::Bool=true,
autodiff = :finite,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a good opportunity to switch to ADTypes instead of Symbols?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point to make the switch for good in v2 not that we are breaking

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened an issue

@pkofod pkofod merged commit 69a18bd into master Nov 1, 2025
21 of 22 checks passed
@pkofod pkofod deleted the pkm/no_kwargs branch November 1, 2025 22:11
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