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.

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

Enable 2D processed variables for arbitrary domains #3200

Closed
brosaplanella opened this issue Jul 28, 2023 · 1 comment · Fixed by #3234
Closed

Enable 2D processed variables for arbitrary domains #3200

brosaplanella opened this issue Jul 28, 2023 · 1 comment · Fixed by #3234
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature in-progress Assigned in the core dev monthly meeting priority: low No existing plans to resolve

Comments

@brosaplanella
Copy link
Member

Description

At the moment, 2D processed variables use a series of if statements to check the domains on which to plot things, which end throwing an error if none of the cases is recognised.

# Process r-x, x-z, r-R, R-x, or R-z
if self.domain[0].endswith("particle") and self.domains["secondary"][
0
].endswith("electrode"):
self.first_dimension = "r"
self.second_dimension = "x"
self.r_sol = first_dim_pts
self.x_sol = second_dim_pts
elif self.domain[0] in [
"negative electrode",
"separator",
"positive electrode",
] and self.domains["secondary"] == ["current collector"]:
self.first_dimension = "x"
self.second_dimension = "z"
self.x_sol = first_dim_pts
self.z_sol = second_dim_pts
elif self.domain[0].endswith("particle") and self.domains["secondary"][
0
].endswith("particle size"):
self.first_dimension = "r"
self.second_dimension = "R"
self.r_sol = first_dim_pts
self.R_sol = second_dim_pts
elif self.domain[0].endswith("particle size") and self.domains["secondary"][
0
].endswith("electrode"):
self.first_dimension = "R"
self.second_dimension = "x"
self.R_sol = first_dim_pts
self.x_sol = second_dim_pts
elif self.domain[0].endswith("particle size") and self.domains["secondary"] == [
"current collector"
]:
self.first_dimension = "R"
self.second_dimension = "z"
self.R_sol = first_dim_pts
self.z_sol = second_dim_pts
else: # pragma: no cover
raise pybamm.DomainError(
f"Cannot process 2D object with domains '{self.domains}'."
)

We should enable a generic case where, if none of the above, the model is plotted taking the secondary domain as horizontal axis and the primary domain as vertical axis (to match the batteries case particle-electrode). Potentially, we could let the user modify this.

Motivation

This would enable running non-battery models in PyBaMM and plotting them.

Possible Implementation

Change current else statement to assign self.first_dimension and self.second_dimension from primary and secondary domain variables. It seems r_sol, x_sol et al are only used in testing.

Additional context

No response

@brosaplanella brosaplanella added feature difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve labels Jul 28, 2023
@brosaplanella
Copy link
Member Author

Similarly, in the 1D case, there's already an else case for generic models, even though it defines the first_dimension as "x" so it should probably also take it from the domain variable.

else:
self.first_dimension = "x"
self.x_sol = space

brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 4, 2023
brosaplanella added a commit that referenced this issue Aug 7, 2023
brosaplanella added a commit that referenced this issue Aug 7, 2023
brosaplanella added a commit that referenced this issue Aug 7, 2023
brosaplanella added a commit that referenced this issue Aug 7, 2023
@brosaplanella brosaplanella added the in-progress Assigned in the core dev monthly meeting label Aug 7, 2023
brosaplanella added a commit that referenced this issue Aug 7, 2023
@brosaplanella brosaplanella self-assigned this Aug 7, 2023
brosaplanella added a commit that referenced this issue Aug 8, 2023
brosaplanella added a commit that referenced this issue Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature in-progress Assigned in the core dev monthly meeting priority: low No existing plans to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant