diff --git a/python/utilities/src/dd_table_for_deal.py b/python/utilities/src/dd_table_for_deal.py index 9c3e1c69f..829be241e 100644 --- a/python/utilities/src/dd_table_for_deal.py +++ b/python/utilities/src/dd_table_for_deal.py @@ -138,14 +138,24 @@ def _parse_limit(text: str) -> int: return int(text) -def _parse_cli(argv: list[str]) -> tuple[str, int, int | None] | None: - """Return (deal_arg, vulnerable, limit) or None for help. +def _parse_numthr(text: str) -> int: + if not text.isdigit(): + raise ValueError( + "Invalid --numthr value (use a non-negative integer; 0 = auto)" + ) + return int(text) + - limit is None when unrestricted. Raises ValueError on bad args. +def _parse_cli(argv: list[str]) -> tuple[str, int, int | None, int] | None: + """Return (deal_arg, vulnerable, limit, num_threads) or None for help. + + limit is None when unrestricted. num_threads defaults to 0 (auto). + Raises ValueError on bad args. """ deal: str | None = None vulnerable = 0 limit: int | None = None + num_threads = 0 i = 1 while i < len(argv): arg = argv[i] @@ -163,6 +173,12 @@ def _parse_cli(argv: list[str]) -> tuple[str, int, int | None] | None: limit = _parse_limit(argv[i + 1]) i += 2 continue + if arg in ("-n", "--numthr"): + if i + 1 >= len(argv): + raise ValueError(f"{arg} requires a non-negative integer") + num_threads = _parse_numthr(argv[i + 1]) + i += 2 + continue if arg.startswith("-") and arg != "-": raise ValueError(f"Unknown option: {arg}") if deal is not None: @@ -176,13 +192,13 @@ def _parse_cli(argv: list[str]) -> tuple[str, int, int | None] | None: else: raise ValueError("missing deal argument") - return deal, vulnerable, limit + return deal, vulnerable, limit, num_threads def _print_usage(prog: str) -> None: print( f"Usage: {prog} [--vul none|both|ns|ew|0|1|2|3] [--limit N] " - f"\n" + f"[-n N|--numthr N] \n" f" {prog} -h | --help\n" "\n" "Calculate double-dummy tricks and par for all strains and leads.\n" @@ -192,6 +208,9 @@ def _print_usage(prog: str) -> None: " --vul Vulnerability: none|both|ns|ew or 0|1|2|3" " (default: none)\n" " --limit Solve only the first N unique deals\n" + " -n, --numthr Worker threads for each table solve.\n" + " 0 = auto (hardware concurrency), 1 = sequential.\n" + " (Default: 0)\n" "\n" 'If stdin is not a terminal, PBN is read from stdin (all [Deal "..."] tags).\n' "\n" @@ -200,6 +219,7 @@ def _print_usage(prog: str) -> None: f'5.A95432.7632.K6 AKJ9842.K.T8.J93"\n' f" {prog} --vul ns hands/example.pbn\n" f" {prog} --limit 3 hands/multi_board.pbn\n" + f" {prog} -n 1 hands/example.pbn\n" f" {prog} < hands/example.pbn\n", file=sys.stderr, ) @@ -476,7 +496,7 @@ def main(argv: list[str] | None = None) -> int: _print_usage(prog) return 0 - input_arg, vulnerable, limit = parsed + input_arg, vulnerable, limit, num_threads = parsed try: pbn_deals = _unique_deals(_load_deals(input_arg)) @@ -497,7 +517,7 @@ def main(argv: list[str] | None = None) -> int: deal_count = len(pbn_deals) for deal_no, pbn_deal in enumerate(pbn_deals, start=1): try: - result = calc_all_tables_pbn([pbn_deal]) + result = calc_all_tables_pbn([pbn_deal], max_threads=num_threads) except (ValueError, RuntimeError) as exc: print(f"DDS error: {exc}", file=sys.stderr) return 1 diff --git a/python/utilities/tests/test_dd_table_for_deal_par.py b/python/utilities/tests/test_dd_table_for_deal_par.py index ff54b92d3..b2b68254c 100644 --- a/python/utilities/tests/test_dd_table_for_deal_par.py +++ b/python/utilities/tests/test_dd_table_for_deal_par.py @@ -67,38 +67,73 @@ def test_rejects_unknown_values(self) -> None: class ParseCliTest(unittest.TestCase): def test_deal_only_defaults_vulnerable_to_none(self) -> None: - deal, vulnerable, limit = _parse_cli(["prog", _EXAMPLE_DEAL]) + deal, vulnerable, limit, num_threads = _parse_cli(["prog", _EXAMPLE_DEAL]) self.assertEqual(deal, _EXAMPLE_DEAL) self.assertEqual(vulnerable, 0) self.assertIsNone(limit) + self.assertEqual(num_threads, 0) def test_vul_flag_before_deal(self) -> None: - deal, vulnerable, limit = _parse_cli(["prog", "--vul", "ns", _EXAMPLE_DEAL]) + deal, vulnerable, limit, num_threads = _parse_cli( + ["prog", "--vul", "ns", _EXAMPLE_DEAL] + ) self.assertEqual(deal, _EXAMPLE_DEAL) self.assertEqual(vulnerable, 2) self.assertIsNone(limit) + self.assertEqual(num_threads, 0) def test_vul_flag_after_deal(self) -> None: - deal, vulnerable, limit = _parse_cli(["prog", _EXAMPLE_DEAL, "--vul", "both"]) + deal, vulnerable, limit, num_threads = _parse_cli( + ["prog", _EXAMPLE_DEAL, "--vul", "both"] + ) self.assertEqual(deal, _EXAMPLE_DEAL) self.assertEqual(vulnerable, 1) self.assertIsNone(limit) + self.assertEqual(num_threads, 0) def test_limit_flag(self) -> None: - deal, vulnerable, limit = _parse_cli( + deal, vulnerable, limit, num_threads = _parse_cli( ["prog", "--limit", "3", _EXAMPLE_DEAL] ) self.assertEqual(deal, _EXAMPLE_DEAL) self.assertEqual(vulnerable, 0) self.assertEqual(limit, 3) + self.assertEqual(num_threads, 0) def test_limit_and_vul_together(self) -> None: - deal, vulnerable, limit = _parse_cli( + deal, vulnerable, limit, num_threads = _parse_cli( ["prog", "--vul", "ns", "--limit", "1", "boards.pbn"] ) self.assertEqual(deal, "boards.pbn") self.assertEqual(vulnerable, 2) self.assertEqual(limit, 1) + self.assertEqual(num_threads, 0) + + def test_numthr_long_and_short_flags(self) -> None: + for flag in ("--numthr", "-n"): + with self.subTest(flag=flag): + deal, vulnerable, limit, num_threads = _parse_cli( + ["prog", flag, "4", _EXAMPLE_DEAL] + ) + self.assertEqual(deal, _EXAMPLE_DEAL) + self.assertEqual(vulnerable, 0) + self.assertIsNone(limit) + self.assertEqual(num_threads, 4) + + def test_numthr_zero_means_auto(self) -> None: + deal, vulnerable, limit, num_threads = _parse_cli( + ["prog", "--numthr", "0", _EXAMPLE_DEAL] + ) + self.assertEqual(deal, _EXAMPLE_DEAL) + self.assertEqual(num_threads, 0) + self.assertIsNone(limit) + self.assertEqual(vulnerable, 0) + + def test_rejects_negative_numthr(self) -> None: + for bad in ("-1", "x", ""): + with self.subTest(bad=bad): + with self.assertRaises(ValueError): + _parse_cli(["prog", "--numthr", bad, _EXAMPLE_DEAL]) def test_rejects_non_positive_limit(self) -> None: for bad in ("0", "-1", "x", ""): @@ -128,6 +163,15 @@ def test_usage_documents_limit(self) -> None: _print_usage("prog") self.assertIn("--limit", buf.getvalue()) + def test_usage_documents_numthr(self) -> None: + buf = io.StringIO() + with redirect_stderr(buf): + _print_usage("prog") + text = buf.getvalue() + self.assertIn("--numthr", text) + self.assertIn("-n", text) + self.assertIn("[-n N|--numthr N]", text) + def test_rejects_unknown_flags(self) -> None: with self.assertRaises(ValueError): _parse_cli(["prog", "--oops", _EXAMPLE_DEAL]) @@ -355,6 +399,28 @@ def test_main_passes_vulnerable_to_par(self) -> None: args, kwargs = par_mock.call_args self.assertEqual(kwargs.get("vulnerable", args[1] if len(args) > 1 else None), 3) + def test_main_passes_numthr_to_calc(self) -> None: + fake_tables = { + "tables": [{"res_table": [[0] * 4 for _ in range(5)]}], + } + fake_par = { + "par_score": ["NS 0", "EW 0"], + "par_contracts_string": ["NS:", "EW:"], + } + with mock.patch( + "dd_table_for_deal.calc_all_tables_pbn", return_value=fake_tables + ) as calc_mock, mock.patch( + "dd_table_for_deal.calc_par_from_table", return_value=fake_par + ), mock.patch( + "dd_table_for_deal._print_pbn_hand" + ), mock.patch( + "dd_table_for_deal._print_table" + ), redirect_stdout(io.StringIO()): + rc = main(["dd_table_for_deal", "-n", "2", _EXAMPLE_DEAL]) + + self.assertEqual(rc, 0) + calc_mock.assert_called_once_with([_EXAMPLE_DEAL], max_threads=2) + def test_main_returns_error_when_par_fails(self) -> None: fake_tables = { "tables": [{"res_table": [[0] * 4 for _ in range(5)]}], diff --git a/utilities/src/dd_table_for_deal/dd_table_for_deal.cpp b/utilities/src/dd_table_for_deal/dd_table_for_deal.cpp index 91a9f8676..37c6d74ee 100644 --- a/utilities/src/dd_table_for_deal/dd_table_for_deal.cpp +++ b/utilities/src/dd_table_for_deal/dd_table_for_deal.cpp @@ -37,10 +37,12 @@ namespace { using dd_table_for_deal::PBN_DEAL_MAX; using dd_table_for_deal::apply_deal_limit; +using dd_table_for_deal::calc_dd_table_for_pbn_deal; using dd_table_for_deal::extract_deal_tags; using dd_table_for_deal::format_par_line; using dd_table_for_deal::looks_like_path; using dd_table_for_deal::parse_limit; +using dd_table_for_deal::parse_numthr; using dd_table_for_deal::parse_vulnerable; using dd_table_for_deal::path_is_openable; using dd_table_for_deal::read_pbn_stream; @@ -193,7 +195,8 @@ auto process_deal( std::string const& deal, std::size_t deal_no, std::size_t deal_count, - int vulnerable) -> bool + int vulnerable, + int num_threads) -> bool { DdTableDealPBN tableDealPBN{}; if (deal.size() >= sizeof(tableDealPBN.cards)) @@ -210,7 +213,14 @@ auto process_deal( DdTableResults table; char line[80]; - const int res = CalcDDtablePBN(tableDealPBN, &table); + const int res = calc_dd_table_for_pbn_deal( + tableDealPBN, + num_threads, + &table, + [](DdTableDealPBN table_deal_pbn, DdTableResults * tablep, int threads) + { + return CalcDDtablePBNN(table_deal_pbn, tablep, threads); + }); if (res != RETURN_NO_FAULT) { ErrorMessage(res, line); @@ -239,7 +249,7 @@ static auto print_usage(const char * prog) -> void { fprintf(stderr, "Usage: %s [--vul none|both|ns|ew|0|1|2|3] [--limit N] " - "\n" + "[-n N|--numthr N] \n" " %s -h | --help\n" "\n" "Calculate double-dummy tricks and par for all strains and leads.\n" @@ -249,6 +259,9 @@ static auto print_usage(const char * prog) -> void " --vul Vulnerability: none|both|ns|ew or 0|1|2|3" " (default: none)\n" " --limit Solve only the first N unique deals\n" + " -n, --numthr Worker threads for each table solve.\n" + " 0 = auto (hardware concurrency), 1 = sequential.\n" + " (Default: 0)\n" "\n" "If stdin is not a terminal, PBN is read from stdin (all [Deal \"...\"] tags).\n" "\n" @@ -257,12 +270,14 @@ static auto print_usage(const char * prog) -> void "5.A95432.7632.K6 AKJ9842.K.T8.J93\"\n" " %s --vul ns hands/example.pbn\n" " %s --limit 3 hands/multi_board.pbn\n" + " %s -n 1 hands/example.pbn\n" " %s < hands/example.pbn\n", prog, prog, prog, prog, prog, + prog, prog); } @@ -271,6 +286,7 @@ auto main(int argc, char * argv[]) -> int { const char * input = nullptr; int vulnerable = 0; + int num_threads = 0; std::optional limit; for (int i = 1; i < argc; ++i) @@ -316,6 +332,28 @@ auto main(int argc, char * argv[]) -> int limit = parsed_limit; continue; } + if (strcmp(argv[i], "-n") == 0 || strcmp(argv[i], "--numthr") == 0) + { + if (i + 1 >= argc) + { + fprintf(stderr, "%s requires a non-negative integer\n", argv[i]); + print_usage(argv[0]); + return 1; + } + const char * flag = argv[i]; + const auto parsed_numthr = parse_numthr(argv[++i]); + if (!parsed_numthr) + { + fprintf(stderr, + "Invalid %s value (use a non-negative integer; " + "0 = auto)\n", + flag); + print_usage(argv[0]); + return 1; + } + num_threads = *parsed_numthr; + continue; + } if (argv[i][0] == '-' && strcmp(argv[i], "-") != 0) { fprintf(stderr, "Unknown option: %s\n", argv[i]); @@ -352,7 +390,7 @@ auto main(int argc, char * argv[]) -> int for (std::size_t i = 0; i < deals.size(); ++i) { - if (!process_deal(deals[i], i + 1, deals.size(), vulnerable)) + if (!process_deal(deals[i], i + 1, deals.size(), vulnerable, num_threads)) return 1; } diff --git a/utilities/src/dd_table_for_deal/dd_table_for_deal.hpp b/utilities/src/dd_table_for_deal/dd_table_for_deal.hpp index 308c1525e..d7b4b1d15 100644 --- a/utilities/src/dd_table_for_deal/dd_table_for_deal.hpp +++ b/utilities/src/dd_table_for_deal/dd_table_for_deal.hpp @@ -10,6 +10,7 @@ #pragma once #include +#include #include #include #include @@ -27,6 +28,19 @@ auto parse_vulnerable(std::string_view text) -> std::optional; // Positive deal count, or nullopt if the text is not a valid positive integer. auto parse_limit(std::string_view text) -> std::optional; +// Worker-thread count for CalcDDtablePBNN: 0 = auto, >=1 = explicit cap. +// Nullopt if the text is not a non-negative integer. +auto parse_numthr(std::string_view text) -> std::optional; + +// Invoke `calc` with `table_deal` and `num_threads` (0 = auto). +using CalcDdTablePbnNFn = + std::function; +auto calc_dd_table_for_pbn_deal( + DdTableDealPBN table_deal, + int num_threads, + DdTableResults * table, + CalcDdTablePbnNFn const& calc) -> int; + // Keep the first `limit` deals when set; otherwise return deals unchanged. auto apply_deal_limit( std::vector deals, diff --git a/utilities/src/dd_table_for_deal/dd_table_for_deal_lib.cpp b/utilities/src/dd_table_for_deal/dd_table_for_deal_lib.cpp index 98ee81261..7aae1cade 100644 --- a/utilities/src/dd_table_for_deal/dd_table_for_deal_lib.cpp +++ b/utilities/src/dd_table_for_deal/dd_table_for_deal_lib.cpp @@ -142,6 +142,35 @@ auto parse_limit(std::string_view text) -> std::optional } +auto parse_numthr(std::string_view text) -> std::optional +{ + if (text.empty()) + return std::nullopt; + + int value = 0; + for (char ch : text) + { + if (ch < '0' || ch > '9') + return std::nullopt; + const int digit = ch - '0'; + if (value > (std::numeric_limits::max() - digit) / 10) + return std::nullopt; + value = value * 10 + digit; + } + return value; +} + + +auto calc_dd_table_for_pbn_deal( + DdTableDealPBN table_deal, + int num_threads, + DdTableResults * table, + CalcDdTablePbnNFn const& calc) -> int +{ + return calc(table_deal, table, num_threads); +} + + auto apply_deal_limit( std::vector deals, std::optional limit) -> std::vector diff --git a/utilities/tests/dd_table_for_deal_test.cpp b/utilities/tests/dd_table_for_deal_test.cpp index e1a2b2cd5..cdb7503e8 100644 --- a/utilities/tests/dd_table_for_deal_test.cpp +++ b/utilities/tests/dd_table_for_deal_test.cpp @@ -78,6 +78,52 @@ TEST(ParseLimit, RejectsNonPositiveAndNonNumeric) } +TEST(ParseNumthr, AcceptsZeroAndPositiveIntegers) +{ + EXPECT_EQ(dd_table_for_deal::parse_numthr("0"), 0); + EXPECT_EQ(dd_table_for_deal::parse_numthr("1"), 1); + EXPECT_EQ(dd_table_for_deal::parse_numthr("8"), 8); + EXPECT_EQ( + dd_table_for_deal::parse_numthr("2147483647"), + 2147483647); +} + + +TEST(ParseNumthr, RejectsNegativeAndNonNumeric) +{ + EXPECT_FALSE(dd_table_for_deal::parse_numthr("").has_value()); + EXPECT_FALSE(dd_table_for_deal::parse_numthr("-1").has_value()); + EXPECT_FALSE(dd_table_for_deal::parse_numthr("3x").has_value()); + EXPECT_FALSE(dd_table_for_deal::parse_numthr("1.5").has_value()); + EXPECT_FALSE(dd_table_for_deal::parse_numthr("2147483648").has_value()); +} + + +TEST(CalcDdTableForPbnDeal, ForwardsNumThreadsToSolver) +{ + int seen_threads = -1; + const auto fake_calc = + [&](DdTableDealPBN /*deal*/, DdTableResults * /*table*/, int num_threads) + { + seen_threads = num_threads; + return RETURN_NO_FAULT; + }; + + DdTableDealPBN deal{}; + DdTableResults table{}; + + EXPECT_EQ( + dd_table_for_deal::calc_dd_table_for_pbn_deal(deal, 1, &table, fake_calc), + RETURN_NO_FAULT); + EXPECT_EQ(seen_threads, 1); + + EXPECT_EQ( + dd_table_for_deal::calc_dd_table_for_pbn_deal(deal, 0, &table, fake_calc), + RETURN_NO_FAULT); + EXPECT_EQ(seen_threads, 0); +} + + TEST(ApplyDealLimit, KeepsPrefixWhenLimited) { const std::vector deals{"a", "b", "c"};