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

Dimensions interchanged in 2D processed variable #1554

Closed
DavidMStraub opened this issue Jul 19, 2021 · 2 comments · Fixed by #1556
Closed

Dimensions interchanged in 2D processed variable #1554

DavidMStraub opened this issue Jul 19, 2021 · 2 comments · Fixed by #1556
Assignees

Comments

@DavidMStraub
Copy link
Contributor

Describe the bug

I'm plotting a 2D processed variable and finding that when I specify the spatial variables y and z, they seem to be interchanged.

To Reproduce
Steps to reproduce the behaviour:

import pybamm
options = {"cell geometry": "pouch", "thermal": "x-lumped", "dimensionality": 2, "current collector": "potential pair"}
model = pybamm.lithium_ion.SPMe(options=options)
experiment = pybamm.Experiment(["Discharge with 3 C for 10 seconds"])
solver = pybamm.CasadiSolver("fast")
sim = pybamm.Simulation(model, experiment=experiment, solver=solver)
sim.solve()
sim.plot(["X-averaged Ohmic heating [W.m-3]"], spatial_unit="m")
print(sim.solution["X-averaged Ohmic heating [W.m-3]"](t=10, y=0.147, z=0.137))
print(sim.solution["X-averaged Ohmic heating [W.m-3]"](t=10, y=0.207, z=0.1))

grafik

array(9830.35092739)
array(17052.65235706)

Expected behaviour

The first printed value should be directly on top of the positive tab, which - as expected and shown in the plot - has a larger heat generation. The second printed value, which has the large heat generation, is a (y, z) point on the right edge of the pouch. So it seems that y and z are interchanged.

@DavidMStraub DavidMStraub changed the title Dimensions interachanged in 2D processed variable Dimensions interchanged in 2D processed variable Jul 19, 2021
@rtimms rtimms self-assigned this Jul 20, 2021
rtimms added a commit that referenced this issue Jul 20, 2021
@rtimms
Copy link
Contributor

rtimms commented Jul 20, 2021

Looks like this was an issue with how the entries of variables that were discretised using FEM were indexed. I have opened a PR with a fix. @DavidMStraub could you checkout the branch and double-check this works as expected please?

@DavidMStraub
Copy link
Contributor Author

Yes, looks good (for the case I was considering), thanks!

rtimms added a commit that referenced this issue Jul 21, 2021
rtimms added a commit that referenced this issue Jul 21, 2021
rtimms added a commit that referenced this issue Jul 21, 2021
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.

2 participants