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

Proposal for a Spatial Method: Spectral Volumes #900

Closed
YannickNoelStephanKuhn opened this issue Mar 20, 2020 · 10 comments
Closed

Proposal for a Spatial Method: Spectral Volumes #900

YannickNoelStephanKuhn opened this issue Mar 20, 2020 · 10 comments

Comments

@YannickNoelStephanKuhn
Copy link
Contributor

Spectral Volumes are a higher-order generalization of Finite Volumes. They subdivide the same mesh Finite Volumes would use into so-called Control Volumes. These are used to compute a polynomial in each Finite Volume which is conservative in each Control Volume. Both the polynomial and its integral average over the Finite Volume are a higher-order approximation.

In contrast to higher-order Finite Volumes, which have all sorts of difficult numerical issues, Spectral Volumes are easily incorporated into any (1D, 2D, 3D) mesh. Additionally, they give higher-order approximations for boundary values which give much better resolution of e.g. Butler-Volmer flows. Compared to Orthogonal Collocation or other Spectral Methods, Spectral Volumes have the following additional benefits:

  • finer resolutions don't require an increase in polynomial order => higher numerical stability;
  • arbitrary subdivisions are possible (but Chebyshev nodes are the most stable);
  • they are a conservative method => suited for long-time simulations.

I implemented 2nd-order Spectral Volumes for my own battery code, and while I couldn't see that the convergence rate increases globally as I hoped for, the convergence rate of interfacial processes was indeed higher. My proposal is that I implement Spectral Volumes of arbitrary order in 1D as a Spatial Method for PyBaMM. A discussion about the storage of the additional internal states and possible output of these internal states for plotting would be necessary first, though.
Relevant paper: https://www.sciencedirect.com/science/article/pii/S0021999102970415

TL;DR: Spectral Volumes are the Finite Volume-equivalent to Spectral Methods for Finite Elements. Since they are not a Spectral Method per se, I didn't write this as a reply to issue #565.

@valentinsulzer
Copy link
Member

Hi @YannickNoelStephanKuhn , that sounds great, thanks for suggesting this! I think the easiest way to tackle this will be to create equations of increasing complexity and make sure they get discretised properly. For example:

# define discretisation
# ....
var = pybamm.Variable("var", domain="negative electrode")
disc.set_variable_slices([var])
# first test
eqn = var
eqn_disc = disc.process_symbol(eqn)
# test discretised as expected
# ....
# second test
eqn = pybamm.grad(var)
eqn_disc = disc.process_symbol(eqn)
# test discretised as expected
# ....
# third test
eqn = pybamm.div(pybamm.grad(var))
eqn_disc = disc.process_symbol(eqn)
# test discretised as expected
# ....

There is also more information on how to add a spatial method here, but the section on which methods need to be overwritten is out of date. See also this example notebook.

Let us know if you need help with anything!

@valentinsulzer
Copy link
Member

Hi @YannickNoelStephanKuhn , how's this going? Do you need any help?

@YannickNoelStephanKuhn
Copy link
Contributor Author

Hello @tinosulzer , thank you for your support. I don't need any help yet, it is just that due to the "current events" due to the [redacted] that I didn't have as much time for this as I would've liked.

@valentinsulzer
Copy link
Member

Haha, fair enough, no pressure :)

@YannickNoelStephanKuhn
Copy link
Contributor Author

I finally found the time to actually implement this. Now I need some help with validating that it works as intended.

The testing code above doesn't work anymore, or I understood something wrong about it. My minimal (non-)working example:

import pybamm

x = pybamm.SpatialVariable("x", domain="negative electrode")
geometry = {"negative electrode": {x: {"min": 0, "max": 1}}}
submesh_types = {"negative electrode": pybamm.Uniform1DSubMesh}
spatial_methods = {"negative electrode": pybamm.FiniteVolume()}
var_pts = {x: 4}
mesh = pybamm.Mesh(geometry, submesh_types, var_pts)
disc = pybamm.Discretisation(mesh, spatial_methods)

var = pybamm.Variable("var", domain="negative electrode")
disc.set_variable_slices([var])

eqn = var
eqn_disc = disc.process_symbol(eqn)
print(eqn_disc)

eqn = pybamm.grad(var)
eqn_disc = disc.process_symbol(eqn)
print(eqn_disc)

eqn = pybamm.div(pybamm.grad(var))
eqn_disc = disc.process_symbol(eqn)
print(eqn_disc)

It breaks for the div-grad-equation with "pybamm.expression_tree.exceptions.ShapeError: Cannot find shape (original error: dimension mismatch)". Any ideas why that happens?

On another note: alongside my spatial method I implemented a 1D submesh which is the only type of submesh that it is compatible with. This way, I could inherit most of pybamm.FiniteVolume. But that isn't as much of a constraint as it sounds like. Consider the following submesh:

|       |               |       |       |
|_ _ _ _|_ _ _ _ _ _ _ _|_ _ _ _|_ _ _ _|

My spatial method requires that each compartment (in this example, there are four) gets subdivided with Chebyshev collocation points. For convergence order 2, this looks like:

|       |               |       |       |
|_ _|_ _|_ _ _ _|_ _ _ _|_ _|_ _|_ _|_ _|

For convergence order 3, this looks like:

|       |               |       |       |
|_|_ _|_|_ _|_ _ _ _|_ _|_|_ _|_|_|_ _|_|

To me, this seemed like the better trade-off than re-writing most of pybamm.FiniteVolume.

@valentinsulzer
Copy link
Member

Glad you have found time to look at this again!
The reason your example doesn't work is that there are no boundary conditions (which add ghost nodes), so the matrices being multiplied are the wrong shape.
Adding the following line or similar after disc.set_variables will fix it:

disc.bcs = {
    var.id: {
        "left": (pybamm.Scalar(0), "Neumann"),
        "right": (pybamm.Scalar(0), "Dirichlet"),
    }
}

Admittedly, this needs a clearer error message.
Usually, disc.bcs is set automatically when doing disc.process_model by reading model.boundary_conditions

@YannickNoelStephanKuhn
Copy link
Contributor Author

I deleted my last comment, since I discovered the problem myself in the meantime. Sorry if I caused confusion.

I simply included some of the operators that are crucial for applying Neumann conditions in the if branch that only gets triggered if Dirichlet conditions are present. Thus, the dimensions didn't match up, and I had to dig for the wrongly discretised symbol in /discretisation/discretisation.py at line 806. The error message there might be more helpful if it printed the discretised symbol in question.

@valentinsulzer
Copy link
Member

Ok, glad you have it under control :) Feel free to add any more error messages that you feel might be useful or more informative. Let me know if you need any more help.

@YannickNoelStephanKuhn
Copy link
Contributor Author

YannickNoelStephanKuhn commented Aug 4, 2020

I might break the error message for some edge cases in the process, and it would only be useful for someone else who implements a spatial method and can't count dimensions properly. :)
So I just added the method and its submesh in my pull request. It does work, as far as I can tell; I tested it with models and they showed the right results.

And my local flake8 somehow missed some things that your flake8 found, so I fixed them in my fork.

@valentinsulzer
Copy link
Member

Fixed by #1123

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

No branches or pull requests

2 participants