PyBaMM icon indicating copy to clipboard operation
PyBaMM copied to clipboard

Fix SPMe Surface Form Discrepancy

Open vidipsingh opened this issue 8 months ago • 2 comments

Description

This PR resolves the mismatch between surface form = "false" and algebraic in SPMe for single-phase configs, ensuring consistent results. It also improves validation for particle phases = ("1", "1") to avoid unintended overrides.

Fixes: #4910

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

Important checks:

Please confirm the following before marking the PR as ready for review:

  • No style issues: nox -s pre-commit
  • All tests pass: nox -s tests
  • The documentation builds: nox -s doctests
  • Code is commented for hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

vidipsingh avatar Apr 19 '25 12:04 vidipsingh

Hi @valentinsulzer, I've made these changes:

  • Ensured surface form = "false" and "algebraic" match for single-phase SPMe ("particle phases": ("1", "1")) by using delta_phi = phi_s - phi_e in CompositeAlgebraic, with validation, and logging.
  • Fixed the particle phases override bug by defaulting to "false" for single-phase setups.

Is this approach correct? Any feedback would be very helpful!

vidipsingh avatar Apr 19 '25 13:04 vidipsingh

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.11%. Comparing base (1ce4ef2) to head (90a4cb7). Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...amm/models/full_battery_models/lithium_ion/spme.py 87.50% 2 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4986      +/-   ##
===========================================
- Coverage    99.12%   99.11%   -0.01%     
===========================================
  Files          304      305       +1     
  Lines        23576    23637      +61     
===========================================
+ Hits         23369    23428      +59     
- Misses         207      209       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 20 '25 16:04 codecov[bot]