explicit maximum number of iterations for alternate scaling algorithm

polettif · Apr 2, 2024 · 16eb907 · 16eb907
1 parent e6d1d24
commit 16eb907
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git a/R/biproportional.R b/R/biproportional.R
@@ -471,7 +471,12 @@ find_matrix_divisors = function(M, seats_cols, seats_rows, round_func) {
         which.max(x)
     }
 
-    while(!all(c(mc(M,dC,dR) == seats_cols, mr(M,dC,dR) == seats_rows))) {
+    # usually less than 20 iterations are needed
+    max_iter = getOption("proporz_max_iterations", 1000)
+    for(i in seq_len(max_iter)) {
+        if(all(c(mc(M,dC,dR) == seats_cols, mr(M,dC,dR) == seats_rows))) {
+            return(list(cols = dC, rows = dR))
+        }
         # change party divisors
         row_decr = which.min0(mr(M,dC,dR) - seats_rows)
         if(length(row_decr) == 1) {
@@ -506,8 +511,8 @@ find_matrix_divisors = function(M, seats_cols, seats_rows, round_func) {
                 seats_cols[col_incr], round_func)
         }
     }
-
-    return(list(cols = dC, rows = dR))
+    stop("Result is undefined, exceeded maximum number of iterations (", max_iter, ")",
+         call. = FALSE)
 }
 
 #' Find divisor to assign seats

diff --git a/R/utils.R b/R/utils.R
@@ -1,11 +1,11 @@
-bisect = function(f, x1, x2, tol = 1e-9) {
+bisect = function(f, x1, x2, tol = 1e-9, max_iterations = 1000) {
     stopifnot(length(x1) == 1, length(x2) == 1, length(tol) == 1, x1 < x2)
     stopifnot((f(x1) <= 0 && f(x2) >= 0) || (f(x1) >= 0 && f(x2) <= 0))
     stopifnot(!is.infinite(x1), !is.infinite(x2))
     stopifnot(!is.nan(x1), !is.nan(x2))
     stopifnot(x1 >= 0, x2 >= 0)
 
-    for(i in 1:1e6) {
+    for(i in seq_len(max_iterations)) {
         x <- (x1 + x2)/2
         if(f(x) == 0 || (x2-x1) < tol) {
             return(x)
@@ -16,7 +16,7 @@ bisect = function(f, x1, x2, tol = 1e-9) {
             x2 <- x
         }
     }
-    stop("Exceeded maximum number of iterations (1e6)") # nocov
+    stop("Exceeded maximum number of iterations (", max_iterations, ")") # nocov
 }
 
 #' Pivot long data.frame to wide matrix and vice versa

diff --git a/tests/testthat/test-biproportional.R b/tests/testthat/test-biproportional.R
@@ -147,6 +147,10 @@ test_that("undefined result biproportional", {
     expect_error_fixed(biproporz(uri2020$votes_matrix, uri2020$seats_vector, quorum_any(any_district = 0.7)),
                        "Result is undefined, equal quotient for parties: 'CVP', 'SPGB', 'FDP', 'SVP'")
 
+    vm5 = matrix(c(10, 10, 10, 10), 2, 2)
+    expect_error_fixed(biproporz(vm5, c(3,1)),
+                       "Result is undefined, exceeded maximum number of iterations")
+
     # manual fix (actual implementation depends on rules)
     vm4 <- vm6 <- vm
     vm4[4,1] <- vm4[4,1]+1