Fix issue #319

Author: peterrrock2

.circleci/config.yml

@@ -42,6 +42,8 @@ jobs:
             echo "backend: Agg" > "matplotlibrc"
             pytest -v --runslow --cov=gerrychain --junitxml=test-reports/junit.xml tests
             codecov
+
+          no_output_timeout: 20m
           environment:
             PYTHONHASHSEED: "0"
       - store_test_results:

gerrychain/__init__.py

@@ -6,7 +6,10 @@
 from .graph import Graph
 from .partition import GeographicPartition, Partition
 from .updaters.election import Election
-
+    
+# Will need to change this to a logging option later
+# It might be good to see how often this happens 
+warnings.simplefilter("once")
 
 try:
     import geopandas

gerrychain/graph/graph.py

@@ -145,6 +145,18 @@ def from_file(
 
         :returns: The Graph object of the geometries from `filename`.
         :rtype: Graph
+
+        .. Warning::
+
+            This method requires the optional ``geopandas`` dependency.
+            So please install ``gerrychain`` with the ``geo`` extra
+            via the command:
+
+            .. code-block:: console
+
+                pip install gerrychain[geo]
+
+            or install ``geopandas`` separately.
         """
         import geopandas as gp
 

gerrychain/partition/geographic.py

@@ -11,7 +11,8 @@
 
 
 class GeographicPartition(Partition):
-    """A :class:`Partition` with areas, perimeters, and boundary information included.
+    """
+    A :class:`Partition` with areas, perimeters, and boundary information included.
     These additional data allow you to compute compactness scores like
     `Polsby-Popper <https://en.wikipedia.org/wiki/Polsby-Popper_Test>`_.
     """

gerrychain/proposals/tree_proposals.py

@@ -6,11 +6,19 @@
 from ..tree import (
     recursive_tree_part, bipartition_tree, bipartition_tree_random,
     _bipartition_tree_random_all, uniform_spanning_tree,
-    find_balanced_edge_cuts_memoization,
+    find_balanced_edge_cuts_memoization, ReselectException,
 )
 from typing import Callable, Optional, Dict, Union
 
 
+class MetagraphError(Exception):
+    """
+    Raised when the partition we are trying to split is a low degree
+    node in the metagraph.
+    """
+    pass
+
+
 def recom(
     partition: Partition,
     pop_col: str,
@@ -70,26 +78,52 @@ def recom(
     :rtype: Partition
     """
 
-    edge = random.choice(tuple(partition["cut_edges"]))
-    parts_to_merge = (partition.assignment.mapping[edge[0]], partition.assignment.mapping[edge[1]])
-
-    subgraph = partition.graph.subgraph(
-        partition.parts[parts_to_merge[0]] | partition.parts[parts_to_merge[1]]
-    )
+    bad_district_pairs = set()
+    n_parts = len(partition)
+    tot_pairs = n_parts * (n_parts - 1) / 2  # n choose 2
 
     # Try to add the region aware in if the method accepts the weight dictionary
     if 'weight_dict' in signature(method).parameters:
         method = partial(method, weight_dict=weight_dict)
 
-    flips = recursive_tree_part(
-        subgraph.graph,
-        parts_to_merge,
-        pop_col=pop_col,
-        pop_target=pop_target,
-        epsilon=epsilon,
-        node_repeats=node_repeats,
-        method=method,
-    )
+    while len(bad_district_pairs) < tot_pairs:
+        try:
+            while True:
+                edge = random.choice(tuple(partition["cut_edges"]))
+                # Need to sort the tuple so that the order is consistent
+                # in the bad_district_pairs set
+                parts_to_merge = [partition.assignment.mapping[edge[0]],
+                                  partition.assignment.mapping[edge[1]]]
+                parts_to_merge.sort()
+
+                if tuple(parts_to_merge) not in bad_district_pairs:
+                    break
+
+            subgraph = partition.graph.subgraph(
+                partition.parts[parts_to_merge[0]] | partition.parts[parts_to_merge[1]]
+            )
+
+            flips = recursive_tree_part(
+                subgraph.graph,
+                parts_to_merge,
+                pop_col=pop_col,
+                pop_target=pop_target,
+                epsilon=epsilon,
+                node_repeats=node_repeats,
+                method=method,
+            )
+            break
+
+        except Exception as e:
+            if isinstance(e, ReselectException):
+                bad_district_pairs.add(tuple(parts_to_merge))
+                continue
+            else:
+                raise
+
+    if len(bad_district_pairs) == tot_pairs:
+        raise MetagraphError(f"Bipartitioning failed for all {tot_pairs} district pairs."
+                             f"Consider rerunning the chain with a different random seed.")
 
     return partition.flip(flips)
 

gerrychain/tree.py

@@ -35,6 +35,7 @@
 import random
 from collections import deque, namedtuple
 from typing import Any, Callable, Dict, List, Optional, Set, Union, Hashable, Sequence, Tuple
+import warnings
 
 
 def predecessors(h: nx.Graph, root: Any) -> Dict:
@@ -295,6 +296,22 @@ def part_nodes(start):
     return cuts
 
 
+class BipartitionWarning(UserWarning):
+    """
+    Generally raised when it is proving difficult to find a balanced cut.
+    """
+    pass
+
+
+class ReselectException(Exception):
+    """
+    Raised when the algorithm is unable to find a balanced cut after some
+    maximum number of attempts, but the user has allowed the algorithm to
+    reselect the pair of nodes to try and recombine.
+    """
+    pass
+
+
 def bipartition_tree(
     graph: nx.Graph,
     pop_col: str,
@@ -306,7 +323,8 @@ def bipartition_tree(
     weight_dict: Optional[Dict] = None,
     balance_edge_fn: Callable = find_balanced_edge_cuts_memoization,
     choice: Callable = random.choice,
-    max_attempts: Optional[int] = 10000
+    max_attempts: Optional[int] = 10000,
+    allow_pair_reselection: bool = False
 ) -> Set:
     """
     This function finds a balanced 2 partition of a graph by drawing a
@@ -347,10 +365,15 @@ def bipartition_tree(
     :param max_attempts: The maximum number of attempts that should be made to bipartition.
         Defaults to 1000.
     :type max_attempts: Optional[int], optional
+    :param allow_pair_reselection: Whether we would like to return an error to the calling
+        function to ask it to reselect the pair of nodes to try and recombine. Defaults to False.
+    :type allow_pair_reselection: bool, optional
 
     :returns: A subset of nodes of ``graph`` (whose induced subgraph is connected). The other
         part of the partition is the complement of this subset.
     :rtype: Set
+
+    :raises BipartitionWarning: If a possible cut cannot be found after 50 attempts.
     :raises RuntimeError: If a possible cut cannot be found after the maximum number of attempts.
     """
     # Try to add the region-aware in if the spanning_tree_fn accepts a weight dictionary
@@ -378,6 +401,17 @@ def bipartition_tree(
         restarts += 1
         attempts += 1
 
+        if attempts == 50 and not allow_pair_reselection:
+            warnings.warn("Failed to find a balanced cut after 50 attempts.\n"
+                          "Consider running with the parameter\n"
+                          "allow_pair_reselection=True to allow the algorithm to\n"
+                          "select a different pair of nodes to try an recombine.",
+                          BipartitionWarning)
+
+    if allow_pair_reselection:
+        raise ReselectException(f"Failed to find a balanced cut after {max_attempts} attempts.\n"
+                                f"Selecting a new district pair")
+
     raise RuntimeError(f"Could not find a possible cut after {max_attempts} attempts.")
 
 
@@ -589,13 +623,17 @@ def recursive_tree_part(
         min_pop = max(pop_target * (1 - epsilon), pop_target * (1 - epsilon) - debt)
         max_pop = min(pop_target * (1 + epsilon), pop_target * (1 + epsilon) - debt)
         new_pop_target = (min_pop + max_pop) / 2
-        nodes = method(
-            graph.subgraph(remaining_nodes),
-            pop_col=pop_col,
-            pop_target=new_pop_target,
-            epsilon=(max_pop - min_pop) / (2 * new_pop_target),
-            node_repeats=node_repeats,
-        )
+
+        try:
+            nodes = method(
+                graph.subgraph(remaining_nodes),
+                pop_col=pop_col,
+                pop_target=new_pop_target,
+                epsilon=(max_pop - min_pop) / (2 * new_pop_target),
+                node_repeats=node_repeats,
+            )
+        except Exception:
+            raise
 
         if nodes is None:
             raise BalanceError()