Number reduced strategies sequentially instead of by action signature

[d-kad]

Description of the changes in this PR

Reduced strategies of extensive games were labelled by concatenating the chosen action's number
at each information set ("*" where unspecified). These labels were not injective — once an information set
had ten or more actions, (1, 11) and (11, 1) both rendered "111" — which broke NFG round-tripping
and made label lookup ambiguous.

This PR labels reduced strategies by their sequence number in generation order ("1", "2", ...),
matching table games. The order is deterministic.

A separate commit fixes unrelated m_behav map pollution found alongside this.

[tturocy]

I've cherry-picked out the sparsity violation and made a separate PR, as consistent use of .at instead of [] is important and we want to fix that irrespective of the disposition of this PR.

I suggest that many of the tests we had with strategy labels were actually testing that we are generating the correct sets of reduced strategies - as we were saying, the crux of the problem is that the strategy label was doing double duty as sometimes communicating details about the reduced strategy mapping of infosets to actions.

So I propose these tests shouldn't just have the strategy labels changed because this is effectively losing coverage. Just as a for-example, test_reduced_strategic_form loses a lot by just changing the strategy labels to be sequential integers, we aren't actually testing the strategy maps at all.

So at least some of these tests should probably be rewritten to exercise strategy.action as appropriate to construct the map and check it's what's expected.

[tturocy]

I have added a commit which proposes some crude UX for the GUI - left-clicking a strategy label in the reduced strategic game pops up a transient window showing the map. It's really basic, but at least we don't lose information for now.

This will tie into the bigger discussion we are having about UX for strategic games (and the GUI in general).

@rahulsavani Noting for you as we should think about what features we might want to explore/display the reduced strategy space, or richer visualisation of the Kuhn's theorem mapping from strategies to behaviours...

[tturocy]

This is a function that actually ought to be part of the game API.

I would suggest something like Game.get_behavior(player: str, strategy: str).

This should return an object which is map-like. We should be able to pass a node or information set (for the moment) to it, and it should return the prescribed action label, or raise an exception if the player does not have the move at that node or information set.

@rahulsavani to think about/discuss at the next meeting.

[rahulsavani]

So I propose these tests shouldn't just have the strategy labels changed because this is effectively losing coverage. Just as a for-example, test_reduced_strategic_form loses a lot by just changing the strategy labels to be sequential integers, we aren't actually testing the strategy maps at all.

So at least some of these tests should probably be rewritten to exercise strategy.action as appropriate to construct the map and check it's what's expected.

Yes, in particular, I worked hard to get the correct old labels in the classes underlying these tests:

        # Centipede
        (games.Centipede.get_test_data(N=3, m0=2, m1=7)),
        (games.Centipede.get_test_data(N=4, m0=2, m1=7)),
        (games.Centipede.get_test_data(N=5, m0=2, m1=7)),
        (games.Centipede.get_test_data(N=3, m0=1, m1=3)),
        (games.Centipede.get_test_data(N=4, m0=1, m1=3)),
        (games.Centipede.get_test_data(N=5, m0=1, m1=3)),
        (games.Centipede.get_test_data(N=9, m0=3, m1=11)),
        # Two player binary tree
        (games.BinEfgTwoPlayer.get_test_data(level=1)),
        (games.BinEfgTwoPlayer.get_test_data(level=2)),
        (games.BinEfgTwoPlayer.get_test_data(level=3)),
        (games.BinEfgTwoPlayer.get_test_data(level=4)),
        (games.BinEfgTwoPlayer.get_test_data(level=5)),
        (games.BinEfgTwoPlayer.get_test_data(level=6)),
        # Three player binary tree
        (games.BinEfgThreePlayer.get_test_data(level=1)),
        (games.BinEfgThreePlayer.get_test_data(level=2)),
        (games.BinEfgThreePlayer.get_test_data(level=3)),
        (games.BinEfgThreePlayer.get_test_data(level=4)),
        (games.BinEfgThreePlayer.get_test_data(level=5)),
        # One player IR binary tree
        (games.BinEfgOnePlayerIR.get_test_data(level=1)),
        (games.BinEfgOnePlayerIR.get_test_data(level=2)),
        (games.BinEfgOnePlayerIR.get_test_data(level=3)),
        (games.BinEfgOnePlayerIR.get_test_data(level=4)),
        (games.BinEfgOnePlayerIR.get_test_data(level=5)),
        (games.BinEfgOnePlayerIR.get_test_data(level=6)),

They seem like natural ones to adapt for still testing that we are doing the strategy map correctly.