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

surface form for SPMe #1207

Closed
rtimms opened this issue Oct 22, 2020 · 3 comments · Fixed by #1257
Closed

surface form for SPMe #1207

rtimms opened this issue Oct 22, 2020 · 3 comments · Fixed by #1257

Comments

@rtimms
Copy link
Contributor

rtimms commented Oct 22, 2020

At the moment if you pass the option {"surface form":"differential"} or {"surface form": "algebraic"} to SPMe nothing happens. SPMe always uses pybamm.electrolyte_conductivity.Composite.

We should either raise an OptionError or add the relevant surface models for SPMe

brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Oct 23, 2020
…lyte conductivity and made it compatible with surface form
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Oct 23, 2020
@brosaplanella
Copy link
Sponsor Member

I tried implementing this in #884 in case it was a quick fix but I am getting errors for both models (full and leading-order). For the moment I set it to throw a NotImplementedError so if that is enough we may close this issue once #884 is merged.

Otherwise, if we want to implement the model, we need to derive the appropriate surface form for Composite. Is there any reference on this kind of formulation?

@valentinsulzer
Copy link
Member

There is my thesis, if you are feeling brave, but I wouldn't go down that route. In theory the leading order form should work though. What was the error?

@brosaplanella
Copy link
Sponsor Member

The differential works but the algebraic doesn't. Here is the error:

======================================================================
ERROR: test_surface_form_algebraic (__main__.TestSPMe)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ferranbrosa/PyBaMM/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme.py", line 119, in test_surface_form_algebraic
    model.check_well_posedness()
  File "/home/ferranbrosa/PyBaMM/pybamm/models/base_model.py", line 377, in check_well_posedness
    self.check_well_determined(post_discretisation)
  File "/home/ferranbrosa/PyBaMM/pybamm/models/base_model.py", line 469, in check_well_determined
    raise pybamm.ModelError("model is overdetermined (repeated keys)")
pybamm.expression_tree.exceptions.ModelError: model is overdetermined (repeated keys)

----------------------------------------------------------------------

rtimms added a commit that referenced this issue Nov 23, 2020
rtimms added a commit that referenced this issue Nov 23, 2020
rtimms added a commit that referenced this issue Nov 26, 2020
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 a pull request may close this issue.

3 participants