python: bind AnalysePlayPBN, AnalyseAllPlaysPBN, DealerPar, SetMaxThreads#160
Open
ThorvaldAagaard wants to merge 3 commits into
Open
python: bind AnalysePlayPBN, AnalyseAllPlaysPBN, DealerPar, SetMaxThreads#160ThorvaldAagaard wants to merge 3 commits into
ThorvaldAagaard wants to merge 3 commits into
Conversation
…eads
Add Python bindings for four DDS C-API functions that had no dds3 wrapper:
- analyse_play_pbn / analyse_all_plays_pbn -> AnalysePlayPBN /
AnalyseAllPlaysPBN, returning {number, tricks} per deal. New
solved_play_to_dict converter.
- dealer_par -> DealerPar, returning {score, number, contracts} from a
calc_dd_table result.
- set_max_threads -> SetMaxThreads, to size the batch-solver thread pool.
All released the GIL around the C call, validate inputs (play length/parity,
seat/vulnerability ranges, board count <= MAXNOOFBOARDS), and reuse the
existing pbn_to_deal / dict_to_dd_table_results helpers. Exposed via
dds3.__init__, documented in docs/python_interface.md, and covered by
python/tests/test_analyse.py (//python:analyse_test). Full suite: 11/11 pass.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Python bindings for DDS play-analysis and dealer-par functionality, exposing additional legacy C-API entry points through the dds3 Python package and documenting/testing them.
Changes:
- Added pybind11 bindings for
AnalysePlayPBN,AnalyseAllPlaysPBN,DealerPar, andSetMaxThreads(asanalyse_play_pbn,analyse_all_plays_pbn,dealer_par,set_max_threads). - Introduced a
SolvedPlay→dictconverter and added Python tests + Bazel test target. - Updated
dds3.__init__exports and extended the Python interface documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| python/tests/test_analyse.py | New unit tests covering the newly-exposed analysis and dealer par APIs. |
| python/src/converters.hpp | Declares the new solved_play_to_dict converter. |
| python/src/converters.cpp | Implements solved_play_to_dict for returning {number, tricks} results to Python. |
| python/src/bindings.cpp | Adds new pybind11 bindings for analysis APIs, dealer par, and thread configuration. |
| python/dds3/init.py | Re-exports the new binding functions from the extension module. |
| python/BUILD.bazel | Adds a new py_test target for the analysis tests. |
| docs/python_interface.md | Documents the new Python APIs. |
Contributor
Author
|
Good comments, I will update accordingly |
…ts, validation - set_max_threads: reject negative input (ValueError); reword docs to state the SetMaxThreads C API is deprecated and DDS 3.x runs the legacy batch APIs single-threaded (no multi-threading promise). Same correction in docs/python_interface.md and the analyse_all_plays_pbn docstring. - solved_play_to_dict / dealer_par: report the clamped count as 'number' so len(tricks)/len(contracts) == number always holds, even on out-of-range input. - test_analyse: add set_max_threads(-1) -> ValueError case. Addresses Copilot review comments.
zzcgumn
reviewed
Jun 2, 2026
|
|
||
| Legacy thread-resource hook (wraps the **deprecated** `SetMaxThreads` C API). | ||
|
|
||
| DDS 3.x removed internal batch parallelism: the legacy batch APIs |
Collaborator
There was a problem hiding this comment.
solve_boards_n has been updated to solve batches in parallel, see line 81 in solve_board.cpp.
That said, python applications may have good reasons to handle parallel execution themselves and dds3 makes this possible.
zzcgumn
reviewed
Jun 2, 2026
| }, | ||
| py::arg("user_threads") = 0, | ||
| "Legacy thread-resource hook (wraps the deprecated SetMaxThreads C API).\n\n" | ||
| "DDS 3.x removed internal batch parallelism: the legacy batch APIs run\n" |
Per review: solve_boards_n parallelises SolveAllBoards across hardware threads (solve_board.cpp), so the earlier 'batch APIs run single-threaded' wording was wrong for solve_all_boards_*. Reword set_max_threads and analyse_all_plays_pbn docs: SetMaxThreads is deprecated and does not size the parallel batch pool; solve_all_boards_* parallelise automatically; analyse_all_plays_pbn still runs sequentially.
Contributor
Author
|
One open question while we're here: given set_max_threads no longer influences the parallel batch path, would you rather I drop the set_max_threads binding entirely from this PR? It's effectively vestigial now — I only included it for C-API completeness. Easy to remove if you'd prefer not to expose a deprecated knob. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Python bindings for four DDS C-API functions that had no dds3 wrapper:
All released the GIL around the C call, validate inputs (play length/parity, seat/vulnerability ranges, board count <= MAXNOOFBOARDS), and reuse the existing pbn_to_deal / dict_to_dd_table_results helpers. Exposed via dds3.init, documented in docs/python_interface.md, and covered by python/tests/test_analyse.py (//python:analyse_test). Full suite: 11/11 pass.