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

Pass options to spatial methods #708

Closed
Scottmar93 opened this issue Nov 6, 2019 · 1 comment
Closed

Pass options to spatial methods #708

Scottmar93 opened this issue Nov 6, 2019 · 1 comment
Assignees

Comments

@Scottmar93
Copy link
Contributor

Scottmar93 commented Nov 6, 2019

Summary
Allow more specific versions of the FiniteVolume method to be used (e.g. with linear extrapolation or quadratic extrapolation, use arithmetic mean or harmonic mean when shifting from nodes to edges)

Should we:

  1. Create a new inherited class for each option (e.g. FiniteVolumeLinearExtrapolation) etc
  2. Allow for such options to be applied to specific symbols (e.g. pybamm.surf(c_s, "linear"))
  3. Create a generator class of spatial methods and allow for a dictionary of options to be passed to the spatial method
  4. change the __init__ of spatial methods so that only a dict of options are taken in upon initialisation and then when the object passed to Discretisation we call a second set_up function that has the same functionality as the current __init__ method.

I am most in favor of 4. but 2. probably requires the least changes to the code.

Thoughts?

Motivation
Following from discussion in pull request #707 .

@valentinsulzer
Copy link
Member

Right, I see that issue that there might be a few different options which makes 1 a bad idea. Agreed that 4 sounds like the best option. It should only require a change to one line in discretisation, and a few relevant tests? We could even make mesh an optional input to __init__, (__init__ calls set_up directly if a mesh is provided) so that existing functionality still works, but this might be confusing

@Scottmar93 Scottmar93 removed the meeting label Nov 7, 2019
@Scottmar93 Scottmar93 self-assigned this Nov 7, 2019
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