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

#1554 fix indexing for FEM entries #1556

Merged
merged 4 commits into from
Jul 21, 2021
Merged

#1554 fix indexing for FEM entries #1556

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
acc5305
#1554 fix indexing for FEM entries
rtimms Jul 20, 2021
1bd8ad3
#1554 changelog
rtimms Jul 21, 2021
7d611b5
#1554 remove extra notebooks
rtimms Jul 21, 2021
7b4905d
Merge branch 'develop' into issue-1554-2D-processed-var
rtimms Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions examples/notebooks/models/pouch-cell-model.ipynb
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 2 additions & 10 deletions pybamm/plotting/quick_plot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,11 +562,7 @@ def plot(self, t, dynamic=False):
y_name = list(spatial_vars.keys())[1][0]
x = self.first_dimensional_spatial_variable[key]
y = self.second_dimensional_spatial_variable[key]
# need to transpose if domain is x-z
if self.is_y_z[key] is True:
var = variable(t_in_seconds, **spatial_vars, warn=False)
else:
var = variable(t_in_seconds, **spatial_vars, warn=False).T
var = variable(t_in_seconds, **spatial_vars, warn=False).T
ax.set_xlabel("{} [{}]".format(x_name, self.spatial_unit))
ax.set_ylabel("{} [{}]".format(y_name, self.spatial_unit))
vmin, vmax = self.variable_limits[key]
Expand Down Expand Up @@ -721,11 +717,7 @@ def slider_update(self, t):
else:
x = self.first_dimensional_spatial_variable[key]
y = self.second_dimensional_spatial_variable[key]
# need to transpose if domain is x-z
if self.is_y_z[key] is True:
var = variable(time_in_seconds, **spatial_vars, warn=False)
else:
var = variable(time_in_seconds, **spatial_vars, warn=False).T
var = variable(time_in_seconds, **spatial_vars, warn=False).T
# store the plot and the var data (for testing) as cant access
# z data from QuadMesh or QuadContourSet object
if self.is_y_z[key] is True:
Expand Down
2 changes: 1 addition & 1 deletion pybamm/solvers/processed_variable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ def initialise_2D_scikit_fem(self):
entries[:, :, idx] = np.reshape(
base_var_casadi(t, y, inputs).full(),
[len_y, len_z],
order="F",
order="C",
)
idx += 1

Expand Down
5 changes: 3 additions & 2 deletions tests/unit/test_plotting/test_quick_plot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ def test_plot_2plus1D_spm(self):
quick_plot.slider_update(1)

# check 2D (y,z space) variables update properly for different time units
# Note: these should be the transpose of the entries in the processed variable
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused: the comment says it should be the transpose but below it is not transposed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the test we do, e.g., np.testing.assert_array_almost_equal(qp_data.T, phi_n[:, :, 0]) so that qp_data is transposed

phi_n = solution["Negative current collector potential [V]"].entries

for unit, scale in zip(["seconds", "minutes", "hours"], [1, 60, 3600]):
Expand All @@ -439,12 +440,12 @@ def test_plot_2plus1D_spm(self):
qp_data = quick_plot.plots[("Negative current collector potential [V]",)][
0
][1]
np.testing.assert_array_almost_equal(qp_data, phi_n[:, :, 0])
np.testing.assert_array_almost_equal(qp_data.T, phi_n[:, :, 0])
quick_plot.slider_update(t_eval[-1] / scale)
qp_data = quick_plot.plots[("Negative current collector potential [V]",)][
0
][1]
np.testing.assert_array_almost_equal(qp_data, phi_n[:, :, -1])
np.testing.assert_array_almost_equal(qp_data.T, phi_n[:, :, -1])

with self.assertRaisesRegex(NotImplementedError, "Shape not recognized for"):
pybamm.QuickPlot(solution, ["Negative particle concentration [mol.m-3]"])
Expand Down