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

Use Aliasing API #2503

Merged
merged 25 commits into from
Dec 21, 2024
Merged

Use Aliasing API #2503

merged 25 commits into from
Dec 21, 2024

Conversation

jClugstor
Copy link
Contributor

Checklist

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

Additional context

This makes use of the Aliasing API in SciMLBase SciML/SciMLBase.jl#830

The default of alias_u0 is preserved here, and it isn't breaking because it still respects when alias_u0 is put in as a kwarg to solve.

@jClugstor jClugstor changed the title Use Aliasing API for alias_u0 Use Aliasing API Dec 12, 2024
@jClugstor jClugstor marked this pull request as draft December 12, 2024 18:57
@jClugstor jClugstor marked this pull request as ready for review December 13, 2024 21:31
@jClugstor
Copy link
Contributor Author

This just needs to use to use the version of SciMLBase with the AliasSpecifiers once that's released, then it should work and pass tests.

Comment on lines 188 to 193
# If alias isa Bool, all fields of ODEAliases set to alias
if alias isa Bool
aliases = ODEAliasSpecifier(alias = alias)
elseif isnothing(alias) || alias isa ODEAliasSpecifier
aliases = alias
end
Copy link
Member

Choose a reason for hiding this comment

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

Canonicalize this in the DifEqBase part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean. This is here so that if alias = true or alias = false in solve that gets passed to the ODEAliasSpecifier constructor so that every variable is aliased / not aliased.

@jClugstor
Copy link
Contributor Author

Oh yeah, this will require a release of DiffEqBase so that the alias keyword is accepted.

@jClugstor
Copy link
Contributor Author

@ChrisRackauckas any chance of a new SciMLBase release coming soon? Also, I documented the alias keyword as a common argument in SciMLBase, and every AbstractAliasSpecifier type has docstrings. Should I also put something in DiffEqDocs?

@ChrisRackauckas
Copy link
Member

Done.

Should I also put something in DiffEqDocs?

Yes in the DiffEqBase/solve.jl docstring.

@ChrisRackauckas ChrisRackauckas merged commit 8353139 into SciML:master Dec 21, 2024
108 of 141 checks passed
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.

3 participants