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

SEI + Composite j_sei undefined #4164

Closed
kratman opened this issue Jun 10, 2024 · 3 comments · Fixed by #4163
Closed

SEI + Composite j_sei undefined #4164

kratman opened this issue Jun 10, 2024 · 3 comments · Fixed by #4163
Assignees
@valentinsulzer

Comments

@kratman
Copy link
Contributor

kratman commented Jun 10, 2024

Report from @mpegis:

The PR (#4153) seemed to fix my issue (#4123) when I declared SEI on both Si/Gr phases in Chen2020, but now I am seeing a different error when defining a single SEI on one phase with "none" on the other/cathode.

import pybamm 
model = pybamm.lithium_ion.DFN(
    {
        "particle phases": ("2", "1"),
        "open-circuit potential": (("single", "current sigmoid"), "single"),
        "SEI": (("none","solvent-diffusion limited"),"none")
    }
)
parameters = pybamm.ParameterValues("Chen2020_composite")
experiment = pybamm.Experiment(["Rest for 10 minutes"])
sim = pybamm.Simulation(model, experiment=experiment, parameter_values=parameters)
sol = sim.solve(calc_esoh=False)

Is giving me this error

UnboundLocalError Traceback (most recent call last)
))
--> 182 j_inner = inner_sei_proportion * Arrhenius * j_sei
183 j_outer = (1 - inner_sei_proportion) * Arrhenius * j_sei
185 variables.update(self._get_standard_concentration_variables(variables))

UnboundLocalError: cannot access local variable 'j_sei' where it is not associated with a value

Was curious if I missed another model option or if this is expected behavior?

Thanks!

Originally posted by @mpegis in #4123 (comment)
@kratman kratman changed the title SEI + Composite j_sei undefied SEI + Composite j_sei undefined Jun 10, 2024
@kratman
Copy link
Contributor Author

kratman commented Jun 10, 2024

@parkec3 Do you want to continue looking into this since you worked on the other PR?

@kratman kratman mentioned this issue Jun 10, 2024
Merged
8 tasks
@kratman
Copy link
Contributor Author

kratman commented Jun 10, 2024

@parkec3 Do you want to continue looking into this since you worked on the other PR?

Never mind, @valentinsulzer already made a branch for a fix

@mpegis
Copy link

mpegis commented Jun 10, 2024 •
edited
Loading

I will just add that I am trying to explore whats possible for coupled degradation when were using a composite electrode. It's somewhat unclear if I am just making syntax mistakes or if there are limitations to the current approach for separate SEI models on the anode, or other coupled degradation models (LAM, etc)

So far I have observed a few things

  • SEI porosity changes seem to be missing a coupled variable when using SEI on both phases
  • A single SEI on one of the phases does not work
  • LAM and particle mechanics do not seem to work with a composite electrode - "OptionError: If there are multiple particle phases: 'surface form' cannot be 'false', 'particle size' must be 'single', 'particle' must be 'Fickian diffusion'. Also the following must be 'none': 'particle mechanics', 'loss of active material'"

@valentinsulzer just FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@valentinsulzer valentinsulzer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix degradation options when one of the phases has no degradation pybamm-team/PyBaMM
3 participants
@kratman @valentinsulzer @mpegis