Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

allow multiple depvars in MOLFiniteDifference #377

Merged

Conversation

valentinsulzer
Copy link
Contributor

Fixes #374

@valentinsulzer valentinsulzer marked this pull request as draft April 19, 2021 19:58
@valentinsulzer
Copy link
Contributor Author

What's the philosophy on adding more tests vs editing the existing ones to capture the more complicated case (2 variables instead of 1)?

@ChrisRackauckas
Copy link
Member

Very dependent on the use case. It's good to have some tests that catch a bunch, but also having some nice small tests that are good to copy-paste and play with is also good.

@valentinsulzer valentinsulzer changed the title WIP: allow multiple depvars in MOLFiniteDifference allow multiple depvars in MOLFiniteDifference Apr 20, 2021
@valentinsulzer valentinsulzer marked this pull request as ready for review April 20, 2021 22:22
@@ -31,7 +33,7 @@ function SciMLBase.symbolic_discretize(pdesys::ModelingToolkit.PDESystem,discret

# Build symbolic variables
indices = CartesianIndices(((axes(s)[1] for s in space)...,))
depvars = map(pdesys.depvars) do u
depvarsdisc = map(depvars) do u
Copy link
Member

Choose a reason for hiding this comment

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

what does the name mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discretized - I found it confusing that depvars != pdesys.depvars

Copy link
Member

Choose a reason for hiding this comment

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

good point, yes it was a consequence of writing too fast. Thanks for correcting it.

@ChrisRackauckas ChrisRackauckas merged commit 0d93632 into SciML:master Apr 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discretize MOLFiniteDifference with more than one dependent variable
2 participants