sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.34k stars 452 forks source link

Deprecation of `maximization` within NormalFormGame #18679

Open 8ac3f0f3-bd13-47e7-baee-523ad1646342 opened 9 years ago

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago

This ticket would be for the deprecation of the maximization parameter within the NormalFormGame class which is used be various functions when computing the Nash equilibrium. Some of the reasons include:

  1. Finding a Nash with maximization=False is equivalent to solving an instance with negative payoffs. So in the two player instance, one might as well create an instance of (-A, -B) that way, creating the same instances over and over when you want to compute equilibria using different algorithms.
  2. Technically, the game being represented isn't the game being solved. As a result, any further manipulations or computations which would need to be done on that instance, we would have to create the bimatrix (-A, -B) again.
  3. For n-players, this gets a little bit messier, as we would have to enumerate the exponentially large number of payoffs in the game so as to negate all of them.

Depends on #18536

CC: @drvinceknight @dimpase @nathanncohen @kcrisman

Component: game theory

Author: Tobenna P. Igwe

Branch/Commit: u/ptigwe/dep_maximization @ 122f8e1

Issue created by migration from https://trac.sagemath.org/ticket/18679

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago

Commit: 1910055

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago

Branch: u/ptigwe/dep_maximization

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago

Dependencies: #18536

d3dfe940-4ef4-4242-81e0-6916cce90212 commented 9 years ago
comment:5

Just wondered why this ticket depends on 18536? Otherwise I'm happy to start reviewing it.

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago
comment:6

I had two main options: branch off from #18536 and deprecate it in all functions including the LP solvers; and the second was to deprecate both within this ticket and #18536. I chose the first as it places the deprecation in one ticket.

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago
comment:7

Actually, come to think of it, I could have just deprecated within this ticket and removed maximization from the LP solvers completely.

drvinceknight commented 9 years ago
comment:8

You could probably still do that... You could comment on #18536 to check if Karl has started reviewing and if not set it back to 'needs work'...

kcrisman commented 9 years ago
comment:9

You could probably still do that... You could comment on #18536 to check if Karl has started reviewing and if not set it back to 'needs work'...

I won't be looking at any of this today, anyway.

drvinceknight commented 9 years ago
comment:10

I won't be looking at any of this today, anyway.

Perfect :)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 1910055 to 73b5780

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

60efdc7Fixed '_as_gambit_game' to support 'maximization' parameter
0e300aeFixed indentation and removed incorrect error
313f42cUpdated tests for cbc and PPL
a24c7ddIncluded tests for constant-sum non-zero sum game and included maximization in the LP solver
2f44485Tests for single / multiple Nash equilibria
fb4461cFixed minor error with LP solver
e4107dcUpdated tests for normal form games
c225b92Remove maximization from LP functions as it is going to be deprecated
93229b7Revert "Remove maximization from LP functions as it is going to be deprecated"
73b5780Deprecate maximization from Normal Form Games
8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago
comment:13

There are some changes which were made in the #18536 ticket which would have caused a few merge conflicts. So in order to avoid that, I think it's best to stick with the current setting.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 73b5780 to e93c4b2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

ddd6f7eRenamed '_as_gambit_game' to '_gambit_' and fixed 'INPUT' indentation issues
e93c4b2Merge branch 'gt_extension' into dep_maximization
kcrisman commented 9 years ago
comment:15

Which order should these tickets be reviewed in, if any?

8ac3f0f3-bd13-47e7-baee-523ad1646342 commented 9 years ago
comment:16

18536 should be reviewed first.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from e93c4b2 to 122f8e1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

6e2aae5Merge branch 'develop' into gt_extension
92345ccModified the '_gambit_' function to support n-player games
2c6aee7Update doctests
06d6b4cUpdated doctests of `catalog` to use `enumeration`
122f8e1Merge branch 'gt_extension' into dep_maximization
cheuberg commented 7 years ago
comment:18

The branch no longer merges with current sage releases.