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

"spherical polar" coordinates ignore left boundary condition #1174

Closed
bessman opened this issue Sep 18, 2020 · 10 comments · Fixed by #1175
Closed

"spherical polar" coordinates ignore left boundary condition #1174

bessman opened this issue Sep 18, 2020 · 10 comments · Fixed by #1175
Assignees

Comments

@bessman
Copy link
Contributor

bessman commented Sep 18, 2020

Describe the bug
When using "spherical polar" coordinates, the left boundary condition seems to always be zero flux, regardless of what the user specifies.

To Reproduce
Steps to reproduce the behaviour:

  1. Run the 'Creating Models/2-a-pde-model.ipynb' notebook.
  2. Change the left boundary condition type and/or value to anything.
  3. Rerun notebook.
  4. Final result is the same.

Expected behaviour
The left boundary condition should be user-changeable. Alternatively, if zero flux is intentionally enforced on the left boundary in spherical coordinates, the user should be warned of this fact if they request something else.

@valentinsulzer
Copy link
Member

Thanks for pointing out - seems to be something do with with the multiplication by r**2 which we should be more careful about

@brosaplanella
Copy link
Member

Hi @bessman! I think that for spherical (and cylindrical coordinates) the condition at r=0 should always be a no-flux condition (note that, in reality, this is an artificial boundary that arises due to the parameterization of the domain). What problem are you trying to solve that gives you a BC other than no-flux?

@bessman
Copy link
Contributor Author

bessman commented Sep 21, 2020

I noticed it while working through some of the example notebooks. Specifically, in "Creating Models/2-a-pde-model.ipynb", the left boundary condition type is given as Dirichlet in the code. This should result in an incorrect result, but does not, because the boundary condition is ignored.

As you say, that's not typically going to be a problem when dealing with spherical particles. I just think the behavior should be clearly documented if it is intentional, to avoid surprising the user.

@rtimms
Copy link
Contributor

rtimms commented Sep 21, 2020

thanks @bessman, it looks like this was a typo in the notebook. as discussed, symmetry always gives no flux at r=0 in spherical polars, but it would be good to add a warning if people try to use a different condition

@brosaplanella
Copy link
Member

Cool, I will modify the code so it throws an error if a "left" bc is passed that it is not a no-flux condition.

@brosaplanella brosaplanella self-assigned this Sep 22, 2020
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Sep 22, 2020
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Sep 22, 2020
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Sep 22, 2020
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Sep 22, 2020
@valentinsulzer
Copy link
Member

@all-contributors add @bessman for bugs, examples

@allcontributors
Copy link
Contributor

@tinosulzer

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected token ] in JSON at position 895

@valentinsulzer
Copy link
Member

@all-contributors add @bessman for bugs, examples

@allcontributors
Copy link
Contributor

@tinosulzer

I've put up a pull request to add @bessman! 🎉

@valentinsulzer
Copy link
Member

Good bot

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.

4 participants