dominicprice / endplay

A suite of tools for generation and analysis of bridge deals. Read the documentation at https://endplay.readthedocs.io
MIT License
21 stars 5 forks source link

Deal.to_lin orders hands incorrectly #12

Closed fbaseggio closed 2 years ago

fbaseggio commented 2 years ago

Lins order hands SWNE. As written, your Lin dumper ends up ordering hands Dealer, LHO, partner, [RHO]. This stems from the to_lin() method on Deal. Locally I fixed it as follows. Changes only within [].

def to_lin(self, dealer: Optional[Player] = None, complete_deal: bool = False) -> str:
    """
    Convert a deal to LIN representation as used by BBO

    :param dealer: If provided, append the dealer (in LIN format) to the returned string
    :param complete_deal: If False, omit the last hand from the string. This is the default
        for files created by BBO.
    """
    lin = str(dealer.to_lin())
    lin += self[Player.south].to_lin() + ","
    lin += self[Player.west].to_lin() + ","
    lin += self[Player.north].to_lin() + ","
    if complete_deal:
        lin += self[Player.east].to_lin()
    return lin
dominicprice commented 2 years ago

Hi, thanks for the feedback - I will see if I have some time tonight to fix this; I haven't worked on this repo for a while due to other commitments but if there are any other suggestions/bugs then if you let me know now then I might do some more tidying up while I have the chance :).

fbaseggio commented 2 years ago

I’m fine, having changed it locally, just trying to be helpful.

I’ve been using it for a while (mainly for access to fast DD) without issue, so probably won’t be. If I find something else, I’ll use it as motivation to figure out how to fork and submit a pull request.

There’s probably a round-trip format change test that makes sense.

One question: when I analyze a list of hands, what speed up do you expect vs doing it one at a time? I get about 2x, but hoped I’d get at least the number of cores I have.

Thanks very much for publishing this, it’s been very useful.

Best,

Franco

On Mar 25, 2022, at 2:08 PM, Dominic Price @.***> wrote:

 Hi, thanks for the feedback - I will see if I have some time tonight to fix this; I haven't worked on this repo for a while due to other commitments but if there are any other suggestions/bugs then if you let me know now then I might do some more tidying up while I have the chance :).

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

dominicprice commented 2 years ago

In theory the dds solver should spawn as many threads as you have cores (the code which detect this is here - how reliably it does this may depend on what OS you're on but I haven't examined the code too closely.

dominicprice commented 2 years ago

Oh and it might be limiting the number of threads due to available memory - it is quite greedy as the transposition table can be a few GB large

fbaseggio commented 2 years ago

thanks, I'll play around with it. Probably need to close some Chrome tabs. Or move to the cloud.

On Fri, Mar 25, 2022 at 2:33 PM Dominic Price @.***> wrote:

Oh and it might be limiting the number of threads due to available memory

  • it is quite greedy as the transposition table can be a few GB large

— Reply to this email directly, view it on GitHub https://github.com/dominicprice/endplay/issues/12#issuecomment-1079297170, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG5ILAPI4OYCM5BVTSL3FIDVBYBI3ANCNFSM5RU2NG4A . You are receiving this because you authored the thread.Message ID: @.***>

dominicprice commented 2 years ago

Fixed in 68da862594aab38f3444c0411942cecbed6788a9