Repository: pybamm-team/PyBaMM (python)

Issue:
thermal boundary condition in 1plus1D 
when setting boundary conditions by tab in the 1plus1D models if there is a tab at either end, then the right boundary condition gets overridden by the "no tab" condition


Pull request:
#711 fix bc bug in 1plus1D
# Description
Fixes a bug where boundary conditions were sometimes set incorrectly in 1plus1D models

Fixes #711 

## Type of change

Please add a line in the relevant section of [CHANGELOG.md](https://github.com/pybamm-team/PyBaMM/blob/master/CHANGELOG.md) to document the change (include PR #) - note reverse order of PR #s.

- [ ] New feature (non-breaking change which adds functionality)
- [ ] Optimization (back-end change that speeds up the code)
- [ ] Bug fix (non-breaking change which fixes an issue)


# Key checklist:

- [ ] No style issues: `$ flake8`
- [ ] All tests pass: `$ python run-tests.py --unit`
- [ ] The documentation builds: `$ cd docs` and then `$ make clean; make html`

You can run all three at once, using `$ python run-tests.py --quick`.

## Further checks:

- [ ] Code is commented, particularly in hard-to-understand areas
- [ ] Tests added that prove fix is effective or that feature works


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 066d81d63..51b38d399 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,7 @@
 
 ## Bug fixes
 
+-   Fixed a bug where boundary conditions were sometimes handled incorrectly in 1+1D models ([#713](https://github.com/pybamm-team/PyBaMM/pull/713))
 -   Corrected a sign error in Dirichlet boundary conditions in the Finite Element Method ([#706](https://github.com/pybamm-team/PyBaMM/pull/706))
 -   Passed the correct dimensional temperature to open circuit potential ([#702](https://github.com/pybamm-team/PyBaMM/pull/702))
 -   Added missing temperature dependence in electrolyte and interface submodels ([#698](https://github.com/pybamm-team/PyBaMM/pull/698))
diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py
index 0c1ab0c6a..1e1653a8a 100644
--- a/pybamm/discretisations/discretisation.py
+++ b/pybamm/discretisations/discretisation.py
@@ -381,19 +381,26 @@ class Discretisation(object):
                     symbol, domain
                 )
             )
-
         # Replace keys with "left" and "right" as appropriate for 1D meshes
         if isinstance(mesh, pybamm.SubMesh1D):
-            # replace negative and/or positive tab
+            # send boundary conditions applied on the tabs to "left" or "right"
+            # depending on the tab location stored in the mesh
             for tab in ["negative tab", "positive tab"]:
                 if any(tab in side for side in list(bcs.keys())):
                     bcs[mesh.tabs[tab]] = bcs.pop(tab)
-            # replace no tab
-            if any("no tab" in side for side in list(bcs.keys())):
-                if "left" in list(bcs.keys()):
-                    bcs["right"] = bcs.pop("no tab")  # tab at bottom
-                else:
-                    bcs["left"] = bcs.pop("no tab")  # tab at top
+            # if there was a tab at either end, then the boundary conditions
+            # have now been set on "left" and "right" as required by the spatial
+            # method, so there is no need to further modify the bcs dict
+            if all(side in list(bcs.keys()) for side in ["left", "right"]):
+                pass
+            # if both tabs are located at z=0 then the "right" boundary condition
+            # (at z=1) is the condition for "no tab"
+            elif "left" in list(bcs.keys()):
+                bcs["right"] = bcs.pop("no tab")
+            # else if both tabs are located at z=1, the "left" boundary condition
+            # (at z=0) is the condition for "no tab"
+            else:
+                bcs["left"] = bcs.pop("no tab")
 
         return bcs
 


Test patch:
diff --git a/tests/unit/test_spatial_methods/test_finite_volume.py b/tests/unit/test_spatial_methods/test_finite_volume.py
index cdee8c22e..99e4cec57 100644
--- a/tests/unit/test_spatial_methods/test_finite_volume.py
+++ b/tests/unit/test_spatial_methods/test_finite_volume.py
@@ -1459,6 +1459,41 @@ class TestFiniteVolume(unittest.TestCase):
         div_eqn_disc = disc.process_symbol(div_eqn)
         div_eqn_disc.evaluate(None, y_test)
 
+    def test_neg_pos_bcs(self):
+        # 2d macroscale
+        mesh = get_1p1d_mesh_for_testing()
+        spatial_methods = {
+            "macroscale": pybamm.FiniteVolume,
+            "negative particle": pybamm.FiniteVolume,
+            "positive particle": pybamm.FiniteVolume,
+            "current collector": pybamm.FiniteVolume,
+        }
+        disc = pybamm.Discretisation(mesh, spatial_methods)
+
+        # var
+        var = pybamm.Variable("var", domain="current collector")
+        disc.set_variable_slices([var])
+        # grad
+        grad_eqn = pybamm.grad(var)
+
+        # bcs (on each tab)
+        boundary_conditions = {
+            var.id: {
+                "negative tab": (pybamm.Scalar(1), "Dirichlet"),
+                "positive tab": (pybamm.Scalar(0), "Neumann"),
+                "no tab": (pybamm.Scalar(8), "Dirichlet"),
+            }
+        }
+        disc.bcs = boundary_conditions
+
+        # check after disc that negative tab goes to left and positive tab goes
+        # to right
+        disc.process_symbol(grad_eqn)
+        self.assertEqual(disc.bcs[var.id]["left"][0].id, pybamm.Scalar(1).id)
+        self.assertEqual(disc.bcs[var.id]["left"][1], "Dirichlet")
+        self.assertEqual(disc.bcs[var.id]["right"][0].id, pybamm.Scalar(0).id)
+        self.assertEqual(disc.bcs[var.id]["right"][1], "Neumann")
+
 
 if __name__ == "__main__":
     print("Add -v for more debug output")