fix bug in leading surface form conductivity#4139
Merged
Conversation
kratman
approved these changes
Jun 5, 2024
valentinsulzer
approved these changes
Jun 5, 2024
brosaplanella
approved these changes
Jun 5, 2024
Member
brosaplanella
left a comment
There was a problem hiding this comment.
Looks good, feel free to merge after adding a line to the README.
8 tasks
js1tr3
pushed a commit
to js1tr3/PyBaMM
that referenced
this pull request
Aug 12, 2024
* fix bug in leading surface form conductivity * update changelog --------- Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com> Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
|
Hello Everyone, Even though the bug is fixed, the EIS values seem erroneous for experimental data of the LGM50 Cell if you use the Chen2020 parameter set. Where the resistance at 1 kHX is 0.025 ohm in the datasheet and the EIS we are obtaining from PyBaMM is far different. I have attached the EIS plot that was obtained for 0.0001 Hz to 1 kHz and the code snipped. This result was the same when I used the PyBaMM-EIS tool instead of time domain calculation. Is there any suggestion or comment on this result? import pybamm
import numpy as np
import matplotlib.pyplot as plt
import time as timer
from scipy.fft import fft
# Set up
model = pybamm.lithium_ion.SPMe(options={"surface form": "differential"}, name="SPM")
V = model.variables["Terminal voltage [V]"]
I = model.variables["Current [A]"]
parameter_values = pybamm.ParameterValues("Chen2020")
frequencies = np.logspace(-4, 2, 30)
# Time domain
I = 50 * 1e-3
number_of_periods = 20
samples_per_period = 16
def current_function(t):
return I * pybamm.sin(2 * np.pi * pybamm.InputParameter("Frequency [Hz]") * t)
parameter_values["Current function [A]"] = current_function
start_time = timer.time()
sim = pybamm.Simulation(
model, parameter_values=parameter_values, solver=pybamm.ScipySolver()
)
impedances_time = []
for frequency in frequencies:
# Solve
period = 1 / frequency
dt = period / samples_per_period
t_eval = np.array(range(0, 1 + samples_per_period * number_of_periods)) * dt
sol = sim.solve(t_eval, inputs={"Frequency [Hz]": frequency})
# Extract final two periods of the solution
time = sol["Time [s]"].entries[-3 * samples_per_period - 1 :]
current = sol["Current [A]"].entries[-3 * samples_per_period - 1 :]
voltage = sol["Voltage [V]"].entries[-3 * samples_per_period - 1 :]
# FFT
current_fft = fft(current)
voltage_fft = fft(voltage)
# Get index of first harmonic
idx = np.argmax(np.abs(current_fft))
impedance = -voltage_fft[idx] / current_fft[idx]
impedances_time.append(impedance)
end_time = timer.time()
time_elapsed = end_time - start_time
print("Time domain method: ", time_elapsed, "s")
fig, ax = plt.subplots()
data = np.array(impedances_time)
ax.plot(data.real, -data.imag, "o")
ax_max = max(data.real.max(), -data.imag.max()) * 1.1
plt.axis([0, ax_max, 0, ax_max])
plt.gca().set_aspect("equal", adjustable="box")
ax.set_xlabel(r"$Z_\mathrm{Re}$ [Ohm]")
ax.set_ylabel(r"$-Z_\mathrm{Im}$ [Ohm]")
plt.show()
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description
Fixes a bug where a factor of electrode surface area to volume ratio is missing in the rhs of the
LeadingOrderDifferentialconductivity model.In particular, this fixes the missing semi-circle when simulating EIS
Before fix:

After fix:

Fixes # (issue)
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run(or$ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all(or$ nox -s tests)$ python run-tests.py --doctest(or$ nox -s doctests)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick(or$ nox -s quick).Further checks: