diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 32b81cf61..06a93e3bf 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -201,6 +201,7 @@ set(cockatrice_SOURCES src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_config.cpp src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_config_dialog.cpp src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.cpp + src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.cpp src/interface/widgets/deck_analytics/analyzer_modules/mana_base/mana_base_config.cpp src/interface/widgets/deck_analytics/analyzer_modules/mana_base/mana_base_config_dialog.cpp src/interface/widgets/deck_analytics/analyzer_modules/mana_base/mana_base_widget.cpp diff --git a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.cpp b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.cpp index 90d9f58ef..2f5b59009 100644 --- a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.cpp @@ -6,6 +6,7 @@ #include "../../deck_list_statistics_analyzer.h" #include "deck_list_model.h" #include "draw_probability_config_dialog.h" +#include "hypergeometric_probability.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/deck_list/tree/deck_list_card_node.h" @@ -233,23 +234,3 @@ void DrawProbabilityWidget::updateFilterOptions() resultTable->setItem(row, 2, new QTableWidgetItem(QString::number(probability * 100.0, 'f', 2))); } } - -double DrawProbabilityWidget::hypergeometricProbability(int N, int K, int n, int k) -{ - if (k < 0 || k > n || K > N || n > N) { - return 0.0; - } - - double logP = 0.0; - for (int i = 1; i <= k; ++i) { - logP += qLn(double(K - k + i) / i); - } - for (int i = 1; i <= n - k; ++i) { - logP += qLn(double(N - K - (n - k) + i) / i); - } - for (int i = 1; i <= n; ++i) { - logP -= qLn(double(N - n + i) / i); - } - - return qExp(logP); -} diff --git a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.h b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.h index 7c7bdb50a..759248c2e 100644 --- a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.h +++ b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/draw_probability_widget.h @@ -3,6 +3,7 @@ #include "../../abstract_analytics_panel_widget.h" #include "draw_probability_config.h" +#include "hypergeometric_probability.h" #include @@ -51,7 +52,6 @@ private: QTableWidget *resultTable; - double hypergeometricProbability(int N, int K, int n, int k); double calculateProbability(int totalCards, int copies, int drawn, bool atLeast); }; diff --git a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.cpp b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.cpp new file mode 100644 index 000000000..79d4f185c --- /dev/null +++ b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.cpp @@ -0,0 +1,27 @@ +#include "hypergeometric_probability.h" + +#include + +double hypergeometricProbability(int N, int K, int n, int k) +{ + // Every rejected case here would otherwise reach qLn with a non-positive argument and + // produce NaN. Beyond the obvious out-of-range checks, the two that matter are k > K (you + // cannot draw more targets than exist) and n - k > N - K (drawing k targets would need more + // non-target cards than the deck holds). + if (k < 0 || k > n || k > K || K > N || n > N || n - k > N - K) { + return 0.0; + } + + double logP = 0.0; + for (int i = 1; i <= k; ++i) { + logP += qLn(double(K - k + i) / i); + } + for (int i = 1; i <= n - k; ++i) { + logP += qLn(double(N - K - (n - k) + i) / i); + } + for (int i = 1; i <= n; ++i) { + logP -= qLn(double(N - n + i) / i); + } + + return qExp(logP); +} diff --git a/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.h b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.h new file mode 100644 index 000000000..3c56ea9ac --- /dev/null +++ b/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.h @@ -0,0 +1,12 @@ +#ifndef COCKATRICE_HYPERGEOMETRIC_PROBABILITY_H +#define COCKATRICE_HYPERGEOMETRIC_PROBABILITY_H + +/** @brief Probability of drawing exactly k of K target cards in a hand of n from a population of N. + * + * Returns 0.0 for arguments that cannot describe a draw, which includes k > K: you cannot draw + * more target cards than the deck holds, and asking for it yields log of a negative term rather + * than a probability. + */ +double hypergeometricProbability(int N, int K, int n, int k); + +#endif // COCKATRICE_HYPERGEOMETRIC_PROBABILITY_H diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 02ecf7cc5..87dfbfed4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -274,6 +274,15 @@ cockatrice_add_test( INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src ) +cockatrice_add_test( + NAME + hypergeometric_probability_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability/hypergeometric_probability.cpp + hypergeometric_probability_test.cpp + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/deck_analytics/analyzer_modules/draw_probability +) cockatrice_add_test( NAME lag_monitor_test diff --git a/tests/hypergeometric_probability_test.cpp b/tests/hypergeometric_probability_test.cpp new file mode 100644 index 000000000..4b45dc53b --- /dev/null +++ b/tests/hypergeometric_probability_test.cpp @@ -0,0 +1,83 @@ +/** @file hypergeometric_probability_test.cpp + * @brief Tests for the draw-probability hypergeometric calculation. + * @ingroup Tests + */ + +#include +#include +#include +#include + +// Drawing more target cards than the deck holds has no valid outcome. Without a k > K guard the +// running product reaches qLn of a negative term, so the result is NaN and the widget prints "nan". +TEST(HypergeometricProbability, AskingForMoreCopiesThanTheDeckHoldsIsZero) +{ + EXPECT_DOUBLE_EQ(hypergeometricProbability(60, 1, 7, 3), 0.0); + EXPECT_DOUBLE_EQ(hypergeometricProbability(60, 2, 7, 4), 0.0); + EXPECT_DOUBLE_EQ(hypergeometricProbability(4, 1, 1, 2), 0.0); +} + +// Drawing k targets from n cards needs n - k non-targets, and the deck may not hold that many. +// An 8-card deck with 6 targets leaves 2 non-targets, so drawing 5 cards with 0 targets would +// need 5 of them. The n - k term goes negative, which is the same NaN through a different factor. +TEST(HypergeometricProbability, AskingForMoreNonTargetsThanTheDeckHoldsIsZero) +{ + EXPECT_DOUBLE_EQ(hypergeometricProbability(8, 6, 5, 0), 0.0); + EXPECT_DOUBLE_EQ(hypergeometricProbability(8, 6, 5, 1), 0.0); + EXPECT_DOUBLE_EQ(hypergeometricProbability(10, 9, 8, 0), 0.0); +} + +// k == K is the reachable edge of the previous case: drawing every target card exactly. It must +// stay valid, so the guard cannot be written as k >= K. +TEST(HypergeometricProbability, DrawingEveryCopyIsNotRejected) +{ + // One Forest in a 60-card deck, drawing 7: P(the Forest) = 7/60. + EXPECT_NEAR(hypergeometricProbability(60, 1, 7, 1), 7.0 / 60.0, 1e-12); + EXPECT_GT(hypergeometricProbability(60, 4, 7, 4), 0.0); + // The only way to fill a hand from a deck the size of the hand is to take everything. + EXPECT_NEAR(hypergeometricProbability(4, 2, 4, 2), 1.0, 1e-12); +} + +// k == 0 with n == 0 draws nothing at all, which is certain. +TEST(HypergeometricProbability, DrawingAnEmptyHandIsCertain) +{ + EXPECT_NEAR(hypergeometricProbability(60, 4, 0, 0), 1.0, 1e-12); + EXPECT_NEAR(hypergeometricProbability(4, 2, 0, 0), 1.0, 1e-12); +} + +TEST(HypergeometricProbability, ImpossibleDrawsAreZero) +{ + EXPECT_DOUBLE_EQ(hypergeometricProbability(60, 4, 7, 9), 0.0); // k > n + EXPECT_DOUBLE_EQ(hypergeometricProbability(10, 12, 5, 3), 0.0); // K > N + EXPECT_DOUBLE_EQ(hypergeometricProbability(10, 4, 20, 5), 0.0); // n > N + EXPECT_DOUBLE_EQ(hypergeometricProbability(10, 4, 5, -1), 0.0); // k < 0 +} + +// The sweep that motivated both guards: no argument combination may produce NaN. The upper bound +// is 1.0 plus a few ulps, since the sum of logarithms accumulates rounding error on a certain +// outcome rather than landing on exactly 1. +TEST(HypergeometricProbability, ProbabilitiesAreFiniteAndInRange) +{ + for (int N = 1; N <= 70; N += 7) { + for (int K = 0; K <= N; K += 3) { + for (int n = 0; n <= N; n += 5) { + for (int k = 0; k <= n; ++k) { + const double p = hypergeometricProbability(N, K, n, k); + const std::string at = "N=" + std::to_string(N) + " K=" + std::to_string(K) + + " n=" + std::to_string(n) + " k=" + std::to_string(k); + ASSERT_FALSE(std::isnan(p)) << at; + ASSERT_GE(p, 0.0) << at; + ASSERT_LE(p, 1.0 + 1e-12) << at; + } + } + } + } +} + +// A known value: the chance of drawing both copies of a 2-of-4 card in a 7-card hand from a +// 60-card deck. C(4,2)*C(56,5)/C(60,7). +TEST(HypergeometricProbability, KnownValueMatchesTheClosedForm) +{ + const double expected = (6.0 * 3819816.0) / 386206920.0; + EXPECT_NEAR(hypergeometricProbability(60, 4, 7, 2), expected, 1e-9); +}