From e9d306c1c5b3ea27b76e8a4e177fc3c8339513fb Mon Sep 17 00:00:00 2001 From: Kristofer Munsterhjelm Date: Fri, 13 Sep 2024 16:27:20 +0200 Subject: [PATCH] =?UTF-8?q?Multiwinner:=20fix=20Sainte-Lagu=C3=AB=20index?= =?UTF-8?q?=20bug,=20add=20timed=20output,=20extract=20convex=5Fhull.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main multiwinner program was passing "observed" and "expected" parameters (opinion profile of the elected candidates and the voters respectively) in the wrong order. I didn't spot this because when I started programming the code in 2008, I used the RMS error, where the order of these parameters don't matter. But for the Sainte-Laguë index, they do. (This still doesn't entirely resolve the oddity Toby Pereira informed me of. More investigation is needed.) The multiwinner program now also only dumps method results ever so often, so that it doesn't emit lots of info we don't need anyway. And most of the convex hull code is now in its own .cc file. --- CMakeLists.txt | 1 + src/main/multiwinner.cc | 40 +++++++++--- src/multiwinner/helper/errors.cc | 4 +- src/stats/multiwinner/convex_hull.cc | 91 ++++++++++++++++++++++++++++ src/stats/multiwinner/convex_hull.h | 89 +++------------------------ 5 files changed, 133 insertions(+), 92 deletions(-) create mode 100644 src/stats/multiwinner/convex_hull.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index b34aa6a..4e91ef0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -208,6 +208,7 @@ add_library(qe_multiwinner_methods src/multiwinner/rusty/auxiliary/dsc.cc src/multiwinner/shuntsstv.cc src/multiwinner/stv.cc + src/stats/multiwinner/convex_hull.cc src/stats/multiwinner/mwstats.cc) add_library(quadelect_lib src/common/ballots.cc diff --git a/src/main/multiwinner.cc b/src/main/multiwinner.cc index 7fd9900..7bb3a7a 100644 --- a/src/main/multiwinner.cc +++ b/src/main/multiwinner.cc @@ -52,6 +52,8 @@ #include "hack/msvc_random.h" +#include "tools/time_tools.h" + #include #include #include @@ -254,8 +256,13 @@ double get_error(std::list council, for (auto pos = council.begin(); pos != council.end(); ++pos) { - assert(0 <= *pos && *pos < num_candidates); - assert(!seen[*pos]); + if (*pos >= (size_t)num_candidates) { + throw std::invalid_argument("get_error: Council names out-of-bounds candidates"); + } + + if (seen[*pos]) { + throw std::invalid_argument("get_error: same candidate named twice"); + } seen[*pos] = true; ++seen_size; @@ -265,9 +272,10 @@ double get_error(std::list council, throw std::invalid_argument("get_error: Given council is the wrong size!"); } - return errm(whole_pop_profile, - get_quantized_opinion_profile( - council, population_profiles)); + // Quantized first! + return errm(get_quantized_opinion_profile( + council, population_profiles), + whole_pop_profile); } /* Majoritarian utility (VSE) @@ -764,6 +772,8 @@ std::vector get_multiwinner_methods( } else { e_methods.push_back(multiwinner_stats(std::make_shared(-1, false))); e_methods.push_back(multiwinner_stats(std::make_shared(-1, true))); + e_methods.push_back(multiwinner_stats(std::make_shared(0.1, false))); + e_methods.push_back(multiwinner_stats(std::make_shared(0.1, true))); e_methods.push_back(multiwinner_stats(std::make_shared(0.5, false))); e_methods.push_back(multiwinner_stats(std::make_shared(0.5, true))); e_methods.push_back(multiwinner_stats(std::make_shared(1, false))); @@ -938,6 +948,8 @@ int main(int argc, char * * argv) { // then this could also do all the Pareto stuff I'd like to do. VSE random_dictator_disprop, random_dictator_maj; + time_pt last_printout = get_now(); + for (int idx = 0; idx < maxnum || run_forever; ++idx) { srandom(idx); // So we can replay in the case of bugs. @@ -1067,6 +1079,9 @@ int main(int argc, char * * argv) { std::cout << s_padded("Prop.", 8) << " " << s_padded("VSE", 8) << " Method\n"; + time_pt now = get_now(); + bool did_printout = false; + for (size_t counter = 0; counter < e_methods.size(); ++counter) { if (e_methods[counter].method() == NULL) { continue; @@ -1100,11 +1115,18 @@ int main(int argc, char * * argv) { method_utility[e_methods[counter].get_name()].add_result( cur_random_utility, this_method_utility, cur_best_utility); - // Print it all out. - // TODO: Place a timer here so we only do it so often. + // Print it all out if it's been a while since last time, or + // if we're at the last iteration. + + if (idx == maxnum-1 || secs_elapsed(last_printout, now) > 5) { + cout << e_methods[counter].display_stats( + method_utility[e_methods[counter].get_name()].get()) << endl; + did_printout = true; + } + } - cout << e_methods[counter].display_stats( - method_utility[e_methods[counter].get_name()].get()) << endl; + if (did_printout) { + last_printout = now; } } } diff --git a/src/multiwinner/helper/errors.cc b/src/multiwinner/helper/errors.cc index 9348f0f..e89c26d 100644 --- a/src/multiwinner/helper/errors.cc +++ b/src/multiwinner/helper/errors.cc @@ -57,7 +57,9 @@ double sli(const std::vector & quantized, const (pop_profile[counter] + slack); } - return (sum/(double)sumcount); + // Not mean, just raw + // not that it matters :-) + return sum; } double webster_idx(const std::vector & quantized, diff --git a/src/stats/multiwinner/convex_hull.cc b/src/stats/multiwinner/convex_hull.cc new file mode 100644 index 0000000..7b21d33 --- /dev/null +++ b/src/stats/multiwinner/convex_hull.cc @@ -0,0 +1,91 @@ +// Calculate the convex hull of VSEs. The interface is similar to +// vse_region_mapper's: call add_point or add_points and then update once +// every new point has been added. + +// This currently only supports 2D convex hulls; I'll add a more general +// algorithm later if/when I need it. + +// Uses Andrew's monotone chain algorithm with cross product, as seen on +// https://en.wikibooks.org/wiki/Algorithm_Implementation/Geometry/Convex_hull/Monotone_chain + +#include "convex_hull.h" + +#include +#include +#include + +double cross(const VSE_point_wrapper & O, const VSE_point_wrapper & A, + const VSE_point_wrapper & B) { + + return (A.x() - O.x()) * (B.y() - O.y()) + - (A.y() - O.y()) * (B.x() - O.x()); +} + +void VSE_convex_hull::add_point(const VSE_point & cur_round_VSE) { + for (auto pos = hull.begin(); pos != hull.end(); ++pos) { + + // Copy the cloud point, add the new data, and insert. + VSE_point new_cloud_point = pos->p; + for (size_t i = 0; i < new_cloud_point.size(); ++i) { + new_cloud_point[i].add_last(cur_round_VSE[i]); + } + + new_points.push_back(new_cloud_point); + } +} + +void VSE_convex_hull::add_points(const std::vector & points) { + for (const VSE_point & p: points) { + add_point(p); + } +} + +// Returns a list of points on the convex hull in counter-clockwise order. +// Note: the last point in the returned list is the same as the first one. +std::vector VSE_convex_hull::convex_hull( + std::vector & P) { + + size_t n = P.size(), k = 0; + if (n <= 3) { + return P; + } + + // Early pruning, etc, TODO; we know roughly where each point is, + // too. + + std::cout << "Convex hull: Dealing with " << n << " potential points.\n"; + + std::vector H(2*n); + + // Sort points lexicographically + std::sort(P.begin(), P.end()); + + // Build lower hull + for (size_t i = 0; i < n; ++i) { + while (k >= 2 && cross(H[k-2], H[k-1], P[i]) <= 0) { + k--; + } + H[k++] = P[i]; + } + + // Build upper hull + for (size_t i = n-1, t = k+1; i > 0; --i) { + while (k >= t && cross(H[k-2], H[k-1], P[i-1]) <= 0) { + k--; + } + H[k++] = P[i-1]; + } + + H.resize(k-1); + + std::cout << "Convex hull: reduced to " << k-1 << " points. " << std::endl; + + return H; +} + +void VSE_convex_hull::dump_coordinates(std::ostream & where) const { + + for (auto pos = hull.begin(); pos != hull.end(); ++pos) { + where << pos->x() << " " << pos->y() << "\n"; + } +} diff --git a/src/stats/multiwinner/convex_hull.h b/src/stats/multiwinner/convex_hull.h index 333c8bb..b3b26fc 100644 --- a/src/stats/multiwinner/convex_hull.h +++ b/src/stats/multiwinner/convex_hull.h @@ -5,14 +5,14 @@ // This currently only supports 2D convex hulls; I'll add a more general // algorithm later if/when I need it. +// I'm also going to hold off on quantization for now. It probably needs +// to happen, but I don't know how to do so without suffering compounding +// losses due to inaccuracy. + // Uses Andrew's monotone chain algorithm with cross product, as seen on // https://en.wikibooks.org/wiki/Algorithm_Implementation/Geometry/Convex_hull/Monotone_chain #include "vse.h" -#include -#include - -// TODO: Quantization class VSE_point_wrapper { public: @@ -44,14 +44,6 @@ class VSE_point_wrapper { } }; -double cross(const VSE_point_wrapper & O, const VSE_point_wrapper & A, - const VSE_point_wrapper & B) { - - return (A.x() - O.x()) * (B.y() - O.y()) - - (A.y() - O.y()) * (B.x() - O.x()); -} - - class VSE_convex_hull { private: std::vector convex_hull( @@ -61,25 +53,8 @@ class VSE_convex_hull { std::vector hull; std::vector new_points; - void add_point(const VSE_point & cur_round_VSE) { - for (auto pos = hull.begin(); pos != hull.end(); ++pos) { - - // Copy the cloud point, add the new data, and insert. - VSE_point new_cloud_point = pos->p; - for (size_t i = 0; i < new_cloud_point.size(); ++i) { - new_cloud_point[i].add_last(cur_round_VSE[i]); - } - - new_points.push_back(new_cloud_point); - } - } - - void add_points(const std::vector & points) { - for (const VSE_point & p: points) { - add_point(p); - } - } - + void add_point(const VSE_point & cur_round_VSE); + void add_points(const std::vector & points); VSE_convex_hull() { // Add empty VSE so that add_point will work properly. @@ -92,54 +67,4 @@ class VSE_convex_hull { } void dump_coordinates(std::ostream & where) const; -}; - -// Returns a list of points on the convex hull in counter-clockwise order. -// Note: the last point in the returned list is the same as the first one. -std::vector VSE_convex_hull::convex_hull( - std::vector & P) { - - size_t n = P.size(), k = 0; - if (n <= 3) { - return P; - } - - // Early pruning, etc, TODO; we know roughly where each point is, - // too. - - std::cout << "Convex hull: Dealing with " << n << " potential points.\n"; - - std::vector H(2*n); - - // Sort points lexicographically - sort(P.begin(), P.end()); - - // Build lower hull - for (size_t i = 0; i < n; ++i) { - while (k >= 2 && cross(H[k-2], H[k-1], P[i]) <= 0) { - k--; - } - H[k++] = P[i]; - } - - // Build upper hull - for (size_t i = n-1, t = k+1; i > 0; --i) { - while (k >= t && cross(H[k-2], H[k-1], P[i-1]) <= 0) { - k--; - } - H[k++] = P[i-1]; - } - - H.resize(k-1); - - std::cout << "Convex hull: reduced to " << k-1 << " points. " << std::endl; - - return H; -} - -void VSE_convex_hull::dump_coordinates(std::ostream & where) const { - - for (auto pos = hull.begin(); pos != hull.end(); ++pos) { - where << pos->x() << " " << pos->y() << "\n"; - } -} +}; \ No newline at end of file