Refactor for clarity, add comments.

We were confused by the algorithm, and this seemed generally
undesirable, so we have attempted a refactor for greater clarity.

We've also used the more standard RNG instance pattern for the random
number generation.

The tests are failing, but this seems also to be true of `main`.

Author: matthew-brett

requirements.txt

@@ -0,0 +1,3 @@
+numpy
+requests
+pyaml

scripts/project_selection.py

@@ -11,47 +11,54 @@ def select_proposals_to_fund(budget, funding_limit, proposals, seed=None):
 
     `proposals` is a list of tuples of the form `(name, requested_amount, previous_funding)`
     where `name` is the name of the proposal, `requested_amount` is the amount of funding
-    that proposal requests and `previos_funding` is the amount of funding the project
-    associated with that proposal has received that year.
+    that proposal requests and `previous_funding` is the amount of funding the
+    project associated with that proposal has received that year.
 
     This function will throw and not produce any results if any of its inputs are invalid.
     """
 
-    np.random.seed(seed)
+    # `seed` can be a seed (integer) or a random number generator.
+    rng = np.random.default_rng(seed)
 
-    for p in proposals:
-        if len(p) != 3:
-            raise ValueError("Malformed proposal")
-        if p[1] + p[2] > funding_limit:
+    names = [str(p[0]) for p in proposals]
+    n_proposals = len(proposals)
+    weights = np.zeros(n_proposals)
+    for i, (p_name, p_fund_ask, p_fund_previous) in enumerate(proposals):
+        remaining_limit = funding_limit - p_fund_previous
+        if p_fund_ask > remaining_limit:
             raise ValueError(
-                f'If proposal "{p[0]}" were funded it would receive more than the funding limit this year.'
+                f'If proposal "{p_name}" were funded it would receive more '
+                'than the per-project funding limit this year.'
             )
-
-    names = [str(p[0]) for p in proposals]
-    weights = [(funding_limit - p[2]) / p[1] for p in proposals]
-
-    for w in weights:
-        assert w >= 1  # this should be redundant with the validation check above.
+        # Decrease weight for projects that have already had previous funding.
+        weights[i] = remaining_limit / p_fund_ask
+        assert weights[i] >= 1  # this should be redundant with the validation check above.
 
     if len(set(names)) != len(names):
         raise ValueError("Proposal names are not unique")
 
     funded = []
     budget_remaining = budget
-    temp_budget_remaining = budget + 0
-    while budget_remaining > 0 and len(funded) < len(proposals):
-        total_weight = sum(weights)
+    while budget_remaining > 0 and len(funded) < n_proposals:
+        total_weight = np.sum(weights)
         if total_weight == 0:
             # When all the proposals have been evaluated and there's still budget
             break
-        i = np.random.choice(range(len(weights)), p=[w / total_weight for w in weights])
+        # Select one project using weights.
+        i = rng.choice(n_proposals, p=weights / total_weight)
+        # Implement selection (but dependent on deficit check below).
         weights[i] = 0
         proposal = proposals[i]
         proposal_budget = proposal[1]
-        temp_budget_remaining -= proposal_budget
-        if temp_budget_remaining > -proposal_budget / 2:
-            funded.append(proposal)
-            budget_remaining = temp_budget_remaining
+        remainder_or_deficit = budget_remaining - proposal_budget
+        if remainder_or_deficit < 0:  # We've gone into deficit.
+            if abs(remainder_or_deficit) > proposal_budget / 2:
+                # Deficit is too great, discard project, search for another
+                # causing less deficit.
+                continue
+        # Confirm selection.
+        funded.append(proposal)
+        budget_remaining = budget_remaining - proposal_budget
 
     print("Inputs:")
     print(f"Budget: ${budget}")
@@ -67,7 +74,7 @@ def select_proposals_to_fund(budget, funding_limit, proposals, seed=None):
     print(
         f"Allocated: ${round(budget - budget_remaining, 2)} (${round(abs(budget_remaining), 2)} {'over' if budget_remaining < 0 else 'under'} budget)"
     )
-    print(f"{len(funded)} proposals funded out of {len(proposals)} total proposals in the drawing")
+    print(f"{len(funded)} proposals funded out of {n_proposals} total proposals in the drawing")
     print()
     print("Funded the following projects")
 

scripts/test_project_selection.py

@@ -1,5 +1,4 @@
 import pytest
-import numpy as np
 
 from project_selection import select_proposals_to_fund
 
@@ -20,15 +19,13 @@
     ],
 )
 def test_select_proposals_wrong_input(proposals, errmessage):
-    np.random.seed(2025)
     budget = 5
     funding_limit = 2
     with pytest.raises(ValueError, match=errmessage):
-        select_proposals_to_fund(budget, funding_limit, proposals)
+        select_proposals_to_fund(budget, funding_limit, proposals, seed=2025)
 
 
 def test_select_proposals_all_funds(capfd):
-    np.random.seed(2025)
     budget = 5
     funding_limit = 2
     proposals = [("A", 2, 0), ("B", 1, 0), ("C", 1, 0), ("D", 0.5, 0), ("E", 0.5, 0)]
@@ -55,15 +52,17 @@ def test_select_proposals_all_funds(capfd):
         'Fund "C" for $1 bringing its project\'s annual total to $1.\n'
         'Fund "A" for $2 bringing its project\'s annual total to $2.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result
     assert captured.out == expected_captured
 
 
 def test_select_proposals_more_than_funds(capfd):
-    np.random.seed(2025)
     budget = 5
     funding_limit = 2
     proposals = [
@@ -103,15 +102,17 @@ def test_select_proposals_more_than_funds(capfd):
         'Fund "B" for $1 bringing its project\'s annual total to $1.\n'
         'Fund "A" for $2 bringing its project\'s annual total to $2.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result
     assert captured.out == expected_captured
 
 
 def test_select_proposals_more_than_funds_under(capfd):
-    np.random.seed(2025)
     budget = 4.1
     funding_limit = 2
     proposals = [
@@ -150,15 +151,17 @@ def test_select_proposals_more_than_funds_under(capfd):
         'Fund "D" for $0.5 bringing its project\'s annual total to $0.5.\n'
         'Fund "B" for $1 bringing its project\'s annual total to $1.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result
     assert captured.out == expected_captured
 
 
 def test_select_proposals_more_than_funds_eqweight_zero(capfd):
-    np.random.seed(2025)
     budget = 6
     funding_limit = 2
     proposals = [
@@ -198,15 +201,17 @@ def test_select_proposals_more_than_funds_eqweight_zero(capfd):
         'Fund "C" for $1 bringing its project\'s annual total to $1.\n'
         'Fund "A" for $1 bringing its project\'s annual total to $1.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result
     assert captured.out == expected_captured
 
 
 def test_select_proposals_more_than_funds_eqweight_under(capfd):
-    np.random.seed(2025)
     budget = 5.4
     funding_limit = 2
     proposals = [
@@ -245,15 +250,17 @@ def test_select_proposals_more_than_funds_eqweight_under(capfd):
         'Fund "D" for $1 bringing its project\'s annual total to $1.\n'
         'Fund "C" for $1 bringing its project\'s annual total to $1.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result
     assert captured.out == expected_captured
 
 
 def test_select_proposals_more_than_funds_eqweight_over(capfd):
-    np.random.seed(2025)
     budget = 6.6
     funding_limit = 2
     proposals = [
@@ -294,7 +301,10 @@ def test_select_proposals_more_than_funds_eqweight_over(capfd):
         'Fund "A" for $1 bringing its project\'s annual total to $1.\n'
         'Fund "F" for $1 bringing its project\'s annual total to $1.\n'
     )
-    result = select_proposals_to_fund(budget, funding_limit, proposals)
+    result = select_proposals_to_fund(budget,
+                                      funding_limit,
+                                      proposals,
+                                      seed=2025)
     captured = capfd.readouterr()
 
     assert set(result) == expected_result

test-requirements.txt

@@ -0,0 +1,2 @@
+-r requirements.txt
+pytest