Improve the code.

- Add a docstring.
- Add type hinting.
- Change algorithm from steepest ascent to simple hill climbing.

common.py

@@ -1,46 +1,93 @@
-from typing import List, Tuple
+from typing import Dict, List, Tuple, Union
+
+import matplotlib.pyplot as plt
 import numpy as np
 import numpy.typing as npt
-import matplotlib.pyplot as plt
 
 # Data given by the assignment
-city_coords = {
-    "Barcelona": [2.154007, 41.390205], "Belgrade": [20.46, 44.79], "Berlin": [13.40, 52.52], 
-    "Brussels": [4.35, 50.85], "Bucharest": [26.10, 44.44], "Budapest": [19.04, 47.50],
-    "Copenhagen": [12.57, 55.68], "Dublin": [-6.27, 53.35], "Hamburg": [9.99, 53.55], 
-    "Istanbul": [28.98, 41.02], "Kyiv": [30.52, 50.45], "London": [-0.12, 51.51], 
-    "Madrid": [-3.70, 40.42], "Milan": [9.19, 45.46], "Moscow": [37.62, 55.75],
-    "Munich": [11.58, 48.14], "Paris": [2.35, 48.86], "Prague": [14.42, 50.07],
-    "Rome": [12.50, 41.90], "Saint Petersburg": [30.31, 59.94], "Sofia": [23.32, 42.70],
-    "Stockholm": [18.06, 60.33], "Vienna": [16.36, 48.21], "Warsaw": [21.02, 52.24]}
+city_coords: Dict = {
+    "Barcelona": [2.154007, 41.390205],
+    "Belgrade": [20.46, 44.79],
+    "Berlin": [13.40, 52.52],
+    "Brussels": [4.35, 50.85],
+    "Bucharest": [26.10, 44.44],
+    "Budapest": [19.04, 47.50],
+    "Copenhagen": [12.57, 55.68],
+    "Dublin": [-6.27, 53.35],
+    "Hamburg": [9.99, 53.55],
+    "Istanbul": [28.98, 41.02],
+    "Kyiv": [30.52, 50.45],
+    "London": [-0.12, 51.51],
+    "Madrid": [-3.70, 40.42],
+    "Milan": [9.19, 45.46],
+    "Moscow": [37.62, 55.75],
+    "Munich": [11.58, 48.14],
+    "Paris": [2.35, 48.86],
+    "Prague": [14.42, 50.07],
+    "Rome": [12.50, 41.90],
+    "Saint Petersburg": [30.31, 59.94],
+    "Sofia": [23.32, 42.70],
+    "Stockholm": [18.06, 60.33],
+    "Vienna": [16.36, 48.21],
+    "Warsaw": [21.02, 52.24],
+}
 
-def plot_plan(city_order: List[str]) -> None:
-    europe_map = plt.imread('map.png')
-    fig, ax = plt.subplots(figsize=(10, 10))
-    ax.imshow(europe_map, extent=[-14.56, 38.43, 37.697 + 0.3, 64.344 + 2.0], aspect="auto")
+
+def plot_plan(city_order: Union[List[str], npt.NDArray]) -> None:
+    """A function that plots the circuit given by city_order.
+
+    This function was given in the assignment from 2024.
+
+    Args:
+        city_order (List[str]): A list of cities in the order to be plotted.
+
+    Returns:
+        None
+    """
+
+    europe_map = plt.imread("map.png")
+    _, ax = plt.subplots(figsize=(10, 10))
+    ax.imshow(
+        europe_map, extent=[-14.56, 38.43, 37.697 + 0.3, 64.344 + 2.0], aspect="auto"
+    )
 
     # Map (long, lat) to (x, y) for plotting
     for index in range(len(city_order) - 1):
         current_city_coords = city_coords[city_order[index]]
-        next_city_coords = city_coords[city_order[index+1]]
+        next_city_coords = city_coords[city_order[index + 1]]
         x, y = current_city_coords[0], current_city_coords[1]
-        #Plotting a line to the next city
+        # Plotting a line to the next city
         next_x, next_y = next_city_coords[0], next_city_coords[1]
         plt.plot([x, next_x], [y, next_y])
 
-        plt.plot(x, y, 'ok', markersize=5)
+        plt.plot(x, y, "ok", markersize=5)
         plt.text(x, y, str(index), fontsize=12)
-    #Finally, plotting from last to first city
+    # Finally, plotting from last to first city
     first_city_coords = city_coords[city_order[0]]
     first_x, first_y = first_city_coords[0], first_city_coords[1]
     plt.plot([next_x, first_x], [next_y, first_y])
-    #Plotting a marker and index for the final city
-    plt.plot(next_x, next_y, 'ok', markersize=5)
-    plt.text(next_x, next_y, str(index+1), fontsize=12)
+    # Plotting a marker and index for the final city
+    plt.plot(next_x, next_y, "ok", markersize=5)
+    plt.text(next_x, next_y, str(index + 1), fontsize=12)
     plt.show()
 
+
+def indexes_to_cities(indexes: npt.NDArray, cities: npt.NDArray) -> npt.NDArray:
+    """Create an array of cities from indeces in a specific order.
+
+    Args:
+        indexes (npt.NDArray): An array of city indexes.
+        cities (npt.NDArray): An array of cities.
+
+    Returns:
+        npt.NDArray An array of cities in the same order as given in indexes.
+    """
+
+    return np.array([cities[i] for i in indexes])
+
+
 def read_data(file_path: str) -> Tuple[npt.NDArray, npt.NDArray]:
-    """ Read the city data from a file given and return 2 arrays.
+    """Read the city data from a file given and return 2 arrays.
 
         The data being read should be separated by semicolons,
         and the first line should be a list of cities while the
@@ -55,6 +102,7 @@ def read_data(file_path: str) -> Tuple[npt.NDArray, npt.NDArray]:
         and the data associated with it.
 
     """
+
     cities = None
     data = []
     with open(file_path, "r") as f:
@@ -67,6 +115,7 @@ def read_data(file_path: str) -> Tuple[npt.NDArray, npt.NDArray]:
 
 
 if __name__ == "__main__":
+    # A test program to see that the file is read correctly.
     cities, data = read_data("./european_cities.csv")
     print(cities)
     print(data)

exhaustive_search.py

@@ -1,13 +1,14 @@
 import time
 from itertools import permutations
-from typing import Callable, Tuple
+from typing import Tuple
 
+import numpy as np
 import numpy.typing as npt
 
 from common import plot_plan, read_data
 
 
-def exhaustive_search(distances: npt.NDArray) -> Tuple[float, Tuple]:
+def exhaustive_search(distances: npt.NDArray) -> Tuple[float, npt.NDArray]:
     """An implementation of exhaustive search.
 
         This implementation takes a permutation iterator, then maps each
@@ -19,8 +20,8 @@ def exhaustive_search(distances: npt.NDArray) -> Tuple[float, Tuple]:
         distances (npt.NDArray): An array containing distances to travel.
 
     Returns:
-        A tuple containing the shortest travel distance and its corresponding
-        permutation.
+        Tuple[float, npt.NDArray] A tuple containing the shortest travel 
+        distance and its corresponding permutation.
     """
 
     size = len(distances)
@@ -29,7 +30,7 @@ def exhaustive_search(distances: npt.NDArray) -> Tuple[float, Tuple]:
         map(  # Map the permutation array to contain tuples (distance, permutation)
             lambda perm: (
                 sum([distances[perm[i - 1], perm[i]] for i in range(size)]),
-                perm,
+                np.array(perm),
             ),
             permutations(range(size)),
         ),
@@ -53,7 +54,7 @@ if __name__ == "__main__":
         print(f"time to find solution: {time_elapsed_ms:>12.6f}ms\n")
 
 
-""" Running example
+"""Running example
 
 oblig1 on  main [?] via 🐍 v3.12.6 took 7s
 ❯ python exhaustive_search.py

hill_climbing.py

@@ -1,53 +1,66 @@
+import copy
 from typing import Tuple
 
 import numpy as np
 import numpy.typing as npt
-import copy
 
-from common import plot_plan, read_data
+from common import indexes_to_cities, plot_plan, read_data
 
-original_perm = None
 
 def hill_climbing(distances: npt.NDArray) -> Tuple[float, npt.NDArray]:
-    size = len(distances)
-    perm = np.arange(size)
-    np.random.shuffle(perm)
+    """A simple hill climbing algorithm.
 
-    print(perm)
-    global original_perm
-    original_perm = copy.deepcopy(perm)
-    current_route: float = np.sum([distances[perm[i - 1], perm[i]] for i in range(size)])
-    found_improvement = True
-    while found_improvement:
-        found_improvement = False
-        tmp_improvement: float = current_route
-        improvement_index: int = -1
-        for i in range(size):
-            perm[i - 1], perm[i] = perm[i], perm[i - 1]
-            tmp_route: float = np.sum([distances[perm[i - 1], perm[i]] for i in range(size)])
-            if tmp_route < tmp_improvement:
-                tmp_improvement = tmp_route
-                improvement_index = i
-                found_improvement = True
+    The algorithm starts on a random permutation and attempts to improve
+    the circuit by trying to switch neighboring elements. Each iteration
+    tries to switch adjacent neighbors and sees which one yields the largest
+    improvement.
 
-            perm[i - 1], perm[i] = perm[i], perm[i - 1]
+    Args:
+        distances npt.NDArray: A matrix containing the distances between cities.
 
-        if found_improvement:
-            current_route = tmp_improvement
-            perm[improvement_index - 1], perm[improvement_index] = (
-                perm[improvement_index],
-                perm[improvement_index - 1],
+    Returns:
+        Tuple[float, npt.NDArray] A tuple containing the distance of the
+        solution and the solution itself.
+
+    """
+
+    size: int = len(distances)  # The size of the permutation array
+    perm: npt.NDArray = np.arange(size)  # Create an array from 0..size
+
+    np.random.shuffle(perm)  # Get random permutation
+
+    # Get the distance of the random permutation
+    current_distance: float = np.sum(
+        [distances[perm[i - 1], perm[i]] for i in range(size)]
     )
 
-    print(perm)
+    found_improvement: bool = True
 
-    return (current_route, perm)
+    while found_improvement:
+
+        found_improvement = False  # Assume we haven't found an improvement
+        tmp_distance: float = current_distance
+
+        # Try to find an improvement
+        for i in range(size):
+            perm[[i - 1, i]] = perm[[i, i - 1]]  # Swap i - 1 and i
+
+            tmp_distance: float = np.sum(
+                [distances[perm[i - 1], perm[i]] for i in range(size)]
+            )
+
+            if tmp_distance < current_distance:
+                current_distance = tmp_distance
+                found_improvement = True
+                break
+
+            perm[[i - 1, i]] = perm[[i, i - 1]]  # Swap back i - 1 and i
+
+    return (current_distance, perm)
 
 
 if __name__ == "__main__":
     cities, data = read_data("./european_cities.csv")
-    distance, perm = hill_climbing(data[:10,:10])
-
-    plot_plan(list(map(lambda i: cities[i], list(perm))))
-    plot_plan(list(map(lambda i: cities[i], list(original_perm))))
+    distance, perm = hill_climbing(data[:10, :10])
 
+    plot_plan(indexes_to_cities(perm, cities))