Merge pull request #1556 from pybamm-team/issue-1554-2D-processed-var

#1554 fix indexing for FEM entries

CHANGELOG.md

@@ -27,6 +27,7 @@
 
 ## Bug fixes
 
+-   Fixed a bug where the order of the indexing for the entries of variables discretised using FEM was incorrect ([#1556](https://github.com/pybamm-team/PyBaMM/pull/1556))
 -   Fix broken module import for spyder when running a script twice ([#1555](https://github.com/pybamm-team/PyBaMM/pull/1555))
 -   Fixed ElectrodeSOH model for multi-dimensional simulations ([#1548](https://github.com/pybamm-team/PyBaMM/pull/1548))
 -   Removed the overly-restrictive check "each variable in the algebraic eqn keys must appear in the eqn" ([#1510](https://github.com/pybamm-team/PyBaMM/pull/1510))

pybamm/plotting/quick_plot.py

@@ -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]
@@ -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:

pybamm/solvers/processed_variable.py

@@ -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
 

tests/unit/test_plotting/test_quick_plot.py

@@ -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
         phi_n = solution["Negative current collector potential [V]"].entries
 
         for unit, scale in zip(["seconds", "minutes", "hours"], [1, 60, 3600]):
@@ -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]"])