-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Costs #25
base: main
Are you sure you want to change the base?
Costs #25
Conversation
pybamm_tea/tea.py
Outdated
if pava.get("Initial loss of lithium inventory") is None: | ||
pava["Initial loss of lithium inventory"] = 0 | ||
warnings.warn("Warning: 'Initial loss of lithium inventory' is set to 0.") | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by this point neither should be None, based on the above logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for noticing, I'll remove the condition.
pybamm_tea/tea.py
Outdated
# initialize concentrations based on initial loss of lithium inventory | ||
pava["Initial concentration in negative electrode [mol.m-3]"] = 0 | ||
pava["Initial concentration in positive electrode [mol.m-3]"] = pava.get("Maximum concentration in positive electrode [mol.m-3]") | ||
warnings.warn("Warning: 'Initial concentration in negative electrode [mol.m-3]' and 'Initial concentration in positive electrode [mol.m-3]' are set to 0 and maximum concentration in positive electrode, respectively.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice the initial concentrations won't be 0 and c_max because you will charge/discharge between voltage limits. I think what you are really doing here is saying that the cyclable lithium is equivalent to starting with a fully lithiated cathode and fully delithiated anode. You could then do something like
param = pybamm.LithiumIonParameters()
Q_n = parameter_values.evaluate(param.n.Q_init)
Q_p = parameter_values.evaluate(param.p.Q_init)
Q_Li = Q_p * f # here 0 < f < 1 gives you initial LLI
inputs = {"Q_n": Q_n, "Q_p": Q_p, "Q_Li": Q_Li}
esoh_solver = pybamm.lithium_ion.ElectrodeSOHSolver(parameter_values, param)
sol = esoh_solver.solve(inputs)
c_n_max = parameter_values.evaluate(param.n.prim.c_max)
c_p_max = parameter_values.evaluate(param.p.prim.c_max)
x = sol["x_100"]
y = sol["y_100"]
parameter_values.update(
{
"Initial concentration in negative electrode [mol.m-3]": x * c_n_max,
"Initial concentration in positive electrode [mol.m-3]": y * c_p_max,
},
check_already_exists=False,
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was that if no initial concentrations are supplied, the total lithium inventory is what would be supplied with the maximum concentration in the positive electrode before formation, without considering any losses at this stage. Later the initial concentrations are updated to 100% SoC, so that the TEA parameter-set can directly be used for a discharge simulation. Would you prefer to have the initial lithium inventory set according to 100% SoC (dependent on voltage cut-offs/stoichiometry limits)?
pybamm_tea/tea.py
Outdated
pava["Lithium inventory [mA.h.cm-2]"] = pava.get("Initial lithium inventory [mA.h.cm-2]") - float(pava.get("Initial loss of lithium inventory [mA.h.cm-2]")) | ||
pava["Initial concentration in negative electrode [mol.m-3]"] = pava.get("Initial concentration in negative electrode [mol.m-3]") * (1 - pava.get("Initial loss of lithium inventory")) | ||
pava["Initial concentration in positive electrode [mol.m-3]"] = pava.get("Initial concentration in positive electrode [mol.m-3]") * (1 - pava.get("Initial loss of lithium inventory")) | ||
pava["Initial stoichiometry"] = pava.get("Lithium inventory [mA.h.cm-2]") / ((pava.get("Negative electrode thickness [m]") * pava.get("Negative electrode active material volume fraction") * pava.get("Maximum concentration in negative electrode [mol.m-3]") + pava.get("Positive electrode thickness [m]") * pava.get("Positive electrode active material volume fraction") * pava.get("Maximum concentration in positive electrode [mol.m-3]")) * 96485 / 3.6 / 10000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what people normally mean be "initial stoichiometry"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no clue, but I don't think so, it is an artefact I delete now. (I don't remember exactly how I wanted to use that value.)
pybamm_tea/tea.py
Outdated
# x0, x100 = self.get_stoichiometries(pava, y0, y100) | ||
# update cut-off voltages if voltage curve(s) are provided | ||
# calculate lithium in positive electrode at SOC = 0 | ||
n_pe_0 = y0 * (pava.get("Maximum concentration in positive electrode [mol.m-3]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be cleaner to have already stored the electrode capacities, then you can just get those from the dict instead of repeating the calculation all the time
pybamm_tea/tea.py
Outdated
* 96485 / 3.6 / 10000)) | ||
if x0 < 0 or x100 < 0 or x0 > 1 or x100 > 1: | ||
raise ValueError("Error: Stoichiometry calculation for negative electrode failed.") | ||
if pava.get("Negative electrode OCP [V]") is not None and pava.get("Positive electrode OCP [V]") is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These voltage cut-offs are normally defined first for the cell, e.g. 2.5-4.2V, and then the stoichiometry windows are calculated to respect those voltage limits. This way seems backwards?
pybamm_tea/tea.py
Outdated
""" | ||
Calculate ideal volumetric and gravimetric energy densities on stack level. | ||
""" | ||
stack_ed = {} # stack energy densities dict | ||
pava = None | ||
pava = self.parameter_values # parameter values | ||
pava = dict(self.parameter_values) # parameter values | ||
|
||
# stoichiometries - calculation based on input stoichiometries or cell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a lot of repeated code here and it is hard to follow. can you explain the use case for being able to independently change sto limits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is useful for capacity balancing in case no OCV curves are supplied. I also generally like it to calculate voltage cut-offs based on it, one could also set target capacities based on it. I'll try to shorten the code section and add more comments so that it becomes easier to follow.
pybamm_tea/tea.py
Outdated
else: | ||
raise ValueError("Error: Stoichiometry calculation failed.") | ||
stack_ed["Negative electrode stoichiometry at 0% SoC"] = x0 | ||
stack_ed["Negative electrode stoichiometry at 100% SoC"] = x100 | ||
stack_ed["Positive electrode stoichiometry at 100% SoC"] = y100 | ||
stack_ed["Positive electrode stoichiometry at 0% SoC"] = y0 | ||
|
||
# update initial concentrations in electrodes to SoC = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this consistent wit previous calculations?
it seems like there are multiple places where the same values get calculated, and it is confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, just a few comments. also, why do you use get
everywhere instead of just indexing into the dict? you only really need to use get
if you think the key might not be there and you want to provide a default when it isn't
Calculate emissions, material and production costs