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

Remove "cylindrical" macro geometry? #4856

Open
valentinsulzer opened this issue Feb 17, 2025 · 1 comment · May be fixed by #4875
Open

Remove "cylindrical" macro geometry? #4856

valentinsulzer opened this issue Feb 17, 2025 · 1 comment · May be fixed by #4875

Comments

@valentinsulzer
Copy link
Member

PyBaMM/src/pybamm/geometry/battery_geometry.py

Lines 153 to 164 in 86b00ce

elif form_factor == "cylindrical":
if current_collector_dimension == 0:
geometry["current collector"] = {"r_macro": {"position": 1}}
elif current_collector_dimension == 1:
geometry["current collector"] = {
"r_macro": {"min": geo.r_inner, "max": 1},
}
else:
raise pybamm.GeometryError(
f"Invalid current collector dimension '{current_collector_dimension}' (should be 0 or 1 for "
"a 'cylindrical' battery geometry)"
)

This is not used anywhere within the repo. Is it used by an external example? @rtimms
@rtimms
Copy link
Contributor

rtimms commented Feb 18, 2025

It can be removed. It was meant if we included a cylindrical thermal model or extended this example to use a “real” battery model.

@vidipsingh vidipsingh linked a pull request Feb 25, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove unused cylindrical battery geometry support vidipsingh/PyBaMM
2 participants
@valentinsulzer @rtimms