b-inary / postflop-solver

[Development suspended] An efficient open-source postflop solver library written in Rust
GNU Affero General Public License v3.0
246 stars 97 forks source link

Add Donk Sizings for turn and river #10

Closed dpmaloney closed 2 years ago

dpmaloney commented 2 years ago

Hi,

In an effort to compare this solver with Pio and Jesolver (A third party solver that is much faster than pio) I decided to implement the donk sizing options that pio uses. If you want an example of this in action, check out this page where oskari tammelin (a member of the alberta research group that solved limit holdem) compares his commercial solver Jesolver to piosolver. He uses this file to create his tests, which uses donk sizes of 50% and 1000% on the turn, and only 1000%.

Adding this to your solver decreases the memory needed and provides a more accurate comparison to Jesolver and pio, as well as removing donk bets that most players (and solvers) will never use, but would take up lots of memory when using them in the solve.

Feel free to change anything about the code style/implementation to fit your style as I know that this is not the best way to achieve what I want.

Thanks!

b-inary commented 2 years ago

Thanks for your pull request!

I wasn't sure whether I should add the feature that allows users to specify the size of donk bets, but if there is a demand, then let's add it. I would be happy if the implementation policy would be as follows:

#[derive(Debug, Clone, Default, PartialEq)]
#[cfg_attr(feature = "bincode", derive(Decode, Encode))]
pub struct DonkSizeCandidates {
    pub donk: Vec<BetSize>,
}
pub turn_donk_sizes: Option<DonkSizeCandidates>,
pub river_donk_sizes: Option<DonkSizeCandidates>,

Also, I think the current implementaion is buggy in the following situation:

In this situation, the OOP's bet action in river is called a probe bet, not a donk bet. I prefer the following implementation:

I could make these changes, but I would be happy if you would modify the pull request. Thanks again!

dpmaloney commented 2 years ago

Sounds good, I can do that! Thank you for fixing it up for me, I made a rought attempt and didn't bug test it but good catch! I think it might be useful for people to use to compare with pio as it is a commonly used feature there.

dpmaloney commented 2 years ago

@b-inary How would you like me to deal with the way let (candidates, is_river) = if node.turn == NOT_DEALT { in push_actions works, as candidates would have a different type if we are using DonkSizeCandidates. I am thinking that I should just create a new donk_candidates so as not to muddy up the code.

dpmaloney commented 2 years ago

Hi @b-inary let me know how the changes look!

b-inary commented 2 years ago

I am very sorry for this very late response. Anyway, I'd like to merge this PR. Thanks so much for your contribution!