Skip to content
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.

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

Add nlprob to ODEFunction #800

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

oscardssmith
Copy link
Contributor

No description provided.

@oscardssmith oscardssmith force-pushed the os/ode-nlfunc-support branch 2 times, most recently from f4d8e36 to 3ded92b Compare October 14, 2024 18:54
@oscardssmith oscardssmith changed the title Draft: add nlfunc to ODEFunction Add nlfunc to ODEFunction Oct 16, 2024
@oscardssmith
Copy link
Contributor Author

The downstream CI failures don't make sense to me. It looks like the downstream is only seeing some of the changes from #816, but I don't really understand it. Closing and reopening to see if that makes CI happier.

@ChrisRackauckas
Copy link
Member

Is this ready @oscardssmith

@oscardssmith
Copy link
Contributor Author

no it needs a quick fix that I'll get to this morning. I'll ping when ready.

@oscardssmith
Copy link
Contributor Author

OK I now think this should work.

@@ -289,6 +289,7 @@ the usage of `f`. These include:
based on the sparsity pattern. Defaults to `nothing`, which means a color vector will be
internally computed on demand when required. The cost of this operation is highly dependent
on the sparsity pattern.
- `nlfunc`: a `NonlinearFunction`
Copy link
Member

Choose a reason for hiding this comment

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

A+ documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docs actually added

@ChrisRackauckas
Copy link
Member

Looks good to go but needs a real docstring 😅

@oscardssmith oscardssmith changed the title Add nlfunc to ODEFunction Add nlprob to ODEFunction Oct 29, 2024
@oscardssmith
Copy link
Contributor Author

changed to be a nlprob instead of nlfunc (to be closer to initializeprob).

@ChrisRackauckas ChrisRackauckas merged commit c341819 into SciML:master Oct 30, 2024
29 of 51 checks passed
@ChrisRackauckas
Copy link
Member

It's still not sufficiently documented... but I'm not going to hold it up anymore. Come back to document it when we have a clearer sense of what it is.

@oscardssmith
Copy link
Contributor Author

what else do you want for docs?

@oscardssmith oscardssmith deleted the os/ode-nlfunc-support branch October 30, 2024 15:35
@ChrisRackauckas
Copy link
Member

Define mathematically what that problem is, what its parameters are, etc.

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