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.

Sign up for GitHub

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

Jump to bottom

Bug positive electrode ohmic losses #1406

Closed
brosaplanella opened this issue Feb 27, 2021 · 0 comments · Fixed by #1407
Closed

Bug positive electrode ohmic losses #1406

brosaplanella opened this issue Feb 27, 2021 · 0 comments · Fixed by #1407
Assignees
@brosaplanella

Comments

@brosaplanella
Copy link
Member

Describe the bug
The positive electrode Ohmic losses are positive, but they should be negative. This is because the definition on line 55 of base_electrode.py has the wrong sign compared to eq [8] in Marquis et al. (2019).

To Reproduce
Steps to reproduce the behaviour:

  1. Just run the default DFN model
  2. Plot "X-averaged positive electrode ohmic losses"
  3. See that it is positive

Expected behaviour
The ohmic losses should be negative.
@brosaplanella brosaplanella self-assigned this Feb 27, 2021
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Feb 27, 2021
@brosaplanella
pybamm-team#1406 fixed bug with positive electrode ohmic losses
8cc397b
@brosaplanella brosaplanella mentioned this issue Feb 27, 2021
Merged
8 tasks
brosaplanella added a commit to brosaplanella/PyBaMM that referenced this issue Feb 27, 2021
@brosaplanella
pybamm-team#1406 update CHANGELOG
a7043df
brosaplanella added a commit that referenced this issue Feb 28, 2021
@brosaplanella
Merge pull request #1407 from brosaplanella/issue-1406-bug-positive-e…
9a95deb 
…lectrode-ohmic-losses

#1406 fixed bug with positive electrode ohmic losses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brosaplanella brosaplanella

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#1406 fixed bug with positive electrode ohmic losses brosaplanella/PyBaMM
1 participant
@brosaplanella