Skip to content

Add correct solve and init dispatches #1052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add correct solve and init dispatches #1052

wants to merge 3 commits into from
+146 −4

Conversation

jClugstor
Copy link
Member

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Fixes up the problems with #1049.

Temporarily uses [sources] in Project.toml to test on SciMLBase master.

ChrisRackauckas
ChrisRackauckas reviewed Oct 10, 2025
View reviewed changes
Project.toml
TerminalLoggers = "5d786b92-1e48-4d6f-9151-6b4477ca9bed"

[sources]
SciMLBase = {url = "https://github.com/SciML/SciMLBase.jl", rev = "master"}
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's to run CI on SciMLBase master, since master has the solve dispatches for OptimizationProblem removed, but latest release still has them.

Copy link
Member

Choose a reason for hiding this comment

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

Just say that. Let's do this right, none of this weird workaround stuff.

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 thought doing a release as it is would break Optimization.jl?

ChrisRackauckas
ChrisRackauckas reviewed Oct 10, 2025
View reviewed changes
lib/OptimizationBase/src/solve.jl
end

function __solve(prob::SciMLBase.OptimizationProblem, alg, args...; kwargs...)
function SciMLBase.__solve(prob::SciMLBase.OptimizationProblem, alg, args...; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

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

piracy, don't do indirect.

Copy link
Member Author

Choose a reason for hiding this comment

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

What should I do here instead? Isn't this how other __solve dispatches are set up?

@ChrisRackauckas
Copy link
Member

There is no solve dispatch in SciMLBase to hit...

@jClugstor
Copy link
Member Author

jClugstor commented Oct 10, 2025 •
edited
Loading

Oh I need to change every SciMLBase.__init and SciMLBase.__solve to just __init and __solve don't I?

Should solve and init be CommonSolve.solve and CommonSolve.init, or should I keep those as SciMLBase.init and SciMLBase.solve. DiffEqBase does SciMLBase, and NonlinearSolve does CommonSolve, not sure which is better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisRackauckas ChrisRackauckas ChrisRackauckas left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jClugstor @ChrisRackauckas