[WIP] Minimax search function #71

a1shadows · 2019-03-29T17:24:55Z

Issue #9

Game trait created.

added missing isTerminalState function

minimaxsearch added

a1shadows · 2019-03-29T17:26:38Z

@BusyByte Hey, could you please resolve the conflicts and review the pull request. Thanks.

BusyByte

Some changes needed but looks good overall.
Can you add a test specification (specs2) and/or a scalacheck (property based check) to prove it works?
If there are examples from the book sometimes I'll use those to prove it works.

core/src/main/scala/aima/core/search/adversarial/MinimaxSearch.scala

BusyByte · 2019-03-30T17:26:08Z

@a1shadows sorry something wasn't quite right with my resolution of the merge conflicts, hopefully you can fix it up

BusyByte · 2019-04-13T14:23:38Z

@a1shadows let me know if you have any questions on the above or need help

case a :: Nil used instead to improve consistency with other cases.

Integrated all requested changes and remedied handful of syntactical and logical errors.

a1shadows · 2019-04-14T08:31:37Z

@BusyByte Have tried addressing all the requested changes. Am working on testing the code. Could you please review the changes?

BusyByte

@a1shadows this looks good but still missing a test specification/property based test.

--edit never mind above, I just saw you were working on the testing code still

a1shadows · 2019-04-22T16:09:45Z

@a1shadows this looks good but still missing a test specification/property based test

I'll do the test code. I have my mid-terms till the 28th. Just need a little bit of time. Thanks for your constant guidance.

added test problem for minimax

test spec for minimax

a1shadows · 2019-05-01T12:24:23Z

@BusyByte I've written a test for the function, but I'm having some difficulty coming up with more tests. Could you please review the new code and suggest how I should proceed with the tests?

BusyByte · 2019-05-05T15:48:01Z

@a1shadows fyi travis has broken, I've merge a fix to master for it now:
#72

you'll need to merge master in order to get travis to run your tests

catching up

BusyByte · 2019-05-05T15:53:57Z

core/src/main/scala/aima/core/search/problems/TwoPlyGame.scala

+case class State(stateNumber: Int)  extends AnyVal
+case class Action(stateNumber: Int) extends AnyVal
+
+object TwoPlyGame extends Game[Player, State, Action] {


Might be worth explaining what this game is and if it's in the book at all provide documentation (comments around it and how it works).

Could you please give an example of the format in which I'm supposed to give the comment about the function?

BusyByte · 2019-05-05T15:58:46Z

core/src/test/scala/aima/core/search/adversarial/TwoPlyMinimaxSearchSpec.scala

+  "The two-ply game " should {
+    "should return action to state 1 from state 0" in {
+      MinimaxDecision
+        .minMaxDecision[Player, State, Action](TwoPlyGame, Action(-1))(TwoPlyGame.initialState, TwoPlyGame.Players(1))


Rather than having a special integer value to signify no action why not have action be a sum type

sealed trait Action case object NoAction extends Action final case class MoveToState(s : State) extends Action

BusyByte · 2019-05-05T16:06:32Z

core/src/main/scala/aima/core/search/problems/TwoPlyGame.scala

+/**
+  * @author Aditya Lahiri
+  */
+case class Player(name: String)     extends AnyVal


if there can only be 2 players why not code it that way;

sealed abstract class Player { def name: String } final case class Player1(name: String) extends Player final case class Player2(name: String) extends Player

BusyByte · 2019-05-05T16:08:58Z

core/src/main/scala/aima/core/search/problems/TwoPlyGame.scala

+                         1 -> List(Action(4), Action(5), Action(6)),
+                         2 -> List(Action(7), Action(8), Action(9)),
+                         3 -> List(Action(10), Action(11), Action(12)))
+  val States = Map(0 -> State(0),


Seems like this might go away with changes I outlined below to Action.

BusyByte · 2019-05-05T16:17:15Z

@a1shadows might be worth going through a full game and verifying each player's actions are what you expect and not just the first one and verify you get NoAction at the end.
I left some suggestions above but this seems pretty good so far.

BusyByte · 2019-05-05T16:27:56Z

@a1shadows also since this is adversarial maybe verify who should win and maybe a comment as to why

Pr/2

a1shadows · 2019-06-19T12:11:51Z

I'm getting a ton of errors on the code when I'm running it on the sbt which weren't there earlier. I can't understand them.
They are all of this kind:

aima-scala\core\src\main\scala\aima\core\agent\Actuator.scala:8:16: type parameter Action defined in trait Actuator shadows trait Action defined in package agent. You may want to rename your type parameter, or possibly remove it.

Could you point me to what's going wrong, please?

a1shadows · 2019-06-19T12:13:58Z

Also, would it be possible to set up a gitter channel to discuss the repository? For general queries and the like?

BusyByte · 2019-06-22T21:19:22Z

@a1shadows I've created a gitter channel, you should get an invitation, I think the error you are seeing is because of a name collision. Maybe try upper case A in your type parameter or upper case ACTION
for example.
trait Foo[A] or trait Bar[ACTION]

BusyByte · 2019-06-22T21:37:55Z

@a1shadows maybe you need to do a sbt clean sbt ";clean;test:compile" after you merge master into your branch.
I don't think there is a trait Action defined in package agent in package agent anymore looking at master.

BusyByte · 2019-06-29T11:30:43Z

@a1shadows this still says WIP in the title. Is there anything else you'd like to do before merging this?

a1shadows · 2019-07-01T03:00:44Z

Yeah. I'd like to work a bit on it.

…

On Sat, 29 Jun 2019, 5:00 pm Shawn Garner, ***@***.***> wrote: @a1shadows <https://github.com/a1shadows> this still says WIP in the title. Is there anything else you'd like to do before merging this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#71?email_source=notifications&email_token=AFIGZIQM36IBESICN2TJWGDP45BWHA5CNFSM4HCLQQRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3XFGA#issuecomment-506950296>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFIGZISJXT33CFT5IDIL6M3P45BWHANCNFSM4HCLQQRA> .

a1shadows · 2019-07-15T02:38:00Z

@BusyByte Hey, I was attempting to rewrite the code to give an optimal state for the game at every level as opposed to my earlier code that only gave the next best move, but I just realized that the pseudocode only provides an algorithm for the next best move with the assumption that the "MAX" player is making the move. I am unsure how to proceed from here on. Should I try to write the code for the complete algorithm that can calculate moves for both "MIN" and "MAX" players or should I restrict myself to what the book has provided?

BusyByte · 2019-07-20T23:02:59Z

@a1shadows Looking at the test for the Java version it looks like it should handle both players.

Looking at the Java implementation it just determines which player's turn it is based on the state. So when you call the search function you give it a state so it should just handle it naturally.

I think it selects the max utility of the minimum utility assuming the next player will pick the min. So it doesn't look like the algorithm needs changed to handle both users.

I think your spec may need to change to handle both users.
It looks like the java spec is implementing aima 3e Fig 5.2

Sorry for the delay in response. Work has been pretty hectic lately.

changes

BusyByte · 2020-01-15T19:39:43Z

@a1shadows is this good to go? If you are satisfied I can merge it. It still says WIP in the title so I'm inclined to wait.

a1shadows · 2020-01-15T19:49:21Z

Ahh. Sorry. I've been busy with work. I'll wrap it up within this weekend.

…

On Thu, 16 Jan 2020, 1:09 am Shawn Garner, ***@***.***> wrote: @a1shadows <https://github.com/a1shadows> is this good to go? If you are satisfied I can merge it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#71?email_source=notifications&email_token=AFIGZIVT26T3YQMRU7P2VN3Q55RABA5CNFSM4HCLQQRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJBRPOI#issuecomment-574822329>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFIGZIVP5DUXYEQNCSZFT53Q55RABANCNFSM4HCLQQRA> .

a1shadows · 2020-01-19T13:06:19Z

Still ironing a few things out. I'll let you know when I'm comfortable with the changes. Is that okay?

BusyByte · 2020-01-22T15:09:38Z

@a1shadows yeah, that's fine but please prioritize this as there are some more PRs coming through and I don't want you to have a bunch of merge conflicts to work through since your PR predates them.

catching up

a1shadows · 2020-01-26T08:10:21Z

I'm satisfied with my changes. Can you review and merge?

BusyByte

Some things I'd change but this looks good.

BusyByte · 2020-02-01T21:27:15Z

core/src/main/scala/aima/core/search/adversarial/Game.scala


-final case class UtilityValue(value: Double) extends AnyVal
+final case class UtilityValue(value: Double) extends AnyVal {
+  def <(that: UtilityValue)  = if (this.value < that.value) true else false


Really this should be typeclass

BusyByte · 2020-02-01T21:28:34Z

core/src/main/scala/aima/core/search/adversarial/Game.scala

+  def getUtility(state: S): UtilityValue
 }
+
+trait Player {}


I don't really like having traits with no defined behavior.
I'd just as soon keep these in the definition of the given game so they can be sealed and compile time checked to be total matched.

BusyByte · 2020-03-01T17:20:04Z

core/src/main/scala/aima/core/search/adversarial/MinimaxSearch.scala

+        maxValue(g.result(state, minMaxValue(g.getActions(state))))
+    }
+
+    maxMinValue(g.getActions(g.initialState))


@a1shadows I was looking at this when trying to implement alpha beta search and have a question. Why are you passing in the g.initialState rather than state which is passed into the function? this does not seem to match the algorithm.

a1shadows · 2020-03-09T11:10:25Z

I decided to not leave that decision up to the caller because the correct behavior will change according to the state for which the decision will be made. For some of the nodes, minMaxValue has to be called for the correct decision and not maxMindecision. Would me like me to change the logic? Sorry for the late reply.

…

On Sun, Mar 1, 2020 at 10:50 PM Shawn Garner ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In core/src/main/scala/aima/core/search/adversarial/MinimaxSearch.scala <#71 (comment)>: > + + def maxValue(state: STATE): UtilityValue = { + if (g.isTerminalState(state)) + g.getUtility(state) + else + minValue(g.result(state, maxMinValue(g.getActions(state)))) + } + + def minValue(state: STATE): UtilityValue = { + if (g.isTerminalState(state)) + g.getUtility(state) + else + maxValue(g.result(state, minMaxValue(g.getActions(state)))) + } + + maxMinValue(g.getActions(g.initialState)) @a1shadows <https://github.com/a1shadows> I was looking at this when trying to implement alpha beta search and have a question. Why are you passing in the g.initialState rather than state which is passed into the function? this does not seem to match the algorithm. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#71?email_source=notifications&email_token=AFIGZIXSKVMEYC4KUHB6NODRFKKMLA5CNFSM4HCLQQRKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCXPAWJQ#pullrequestreview-366873382>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFIGZIUA4J4SVH7D6FNHY2DRFKKMLANCNFSM4HCLQQRA> .

a1shadows added 5 commits February 2, 2019 22:54

20f4b6a

Game trait created.

parameter error resolved

b80af13

removed implicit parameter

feec143

b048088

added missing isTerminalState function

1376a59

minimaxsearch added

a1shadows changed the title ~~Minimax search function~~ [WIP]Minimax search function Mar 29, 2019

a1shadows changed the title ~~[WIP]Minimax search function~~ [WIP] Minimax search function Mar 29, 2019

Merge branch 'master' into master

53a2fab

BusyByte requested changes Mar 30, 2019

View reviewed changes

a1shadows added 2 commits April 14, 2019 11:05

1st requested change

84a83ea

case a :: Nil used instead to improve consistency with other cases.

2nd, 3rd, 4th, 5th, 6th requested changes made, errors resolved

d1dd9ec

Integrated all requested changes and remedied handful of syntactical and logical errors.

BusyByte approved these changes Apr 20, 2019

View reviewed changes

a1shadows added 2 commits May 1, 2019 17:49

560a31f

added test problem for minimax

test added

7561f7b

test spec for minimax

Merge pull request #1 from aimacode/master

56e5434

catching up

BusyByte approved these changes May 5, 2019

View reviewed changes

a1shadows added 2 commits June 19, 2019 17:29

Merge branch 'master' into pr/2

53d67b5

Merge pull request #3 from a1shadows/pr/2

0f3efe4

Pr/2

a1shadows added 2 commits August 18, 2019 13:47

reformatting

20d2e6a

shanges

79ec015

changes

review changes

cab3a7f

a1shadows added 3 commits January 26, 2020 13:26

errors fixed, spec added

61509dc

Merge pull request #4 from aimacode/master

ec0a977

catching up

clean

397fc7e

BusyByte approved these changes Feb 1, 2020

View reviewed changes

BusyByte merged commit e4e4b06 into aimacode:master Feb 1, 2020

BusyByte reviewed Mar 1, 2020

View reviewed changes

[WIP] Minimax search function #71

[WIP] Minimax search function #71

Uh oh!

Conversation

a1shadows commented Mar 29, 2019

Uh oh!

a1shadows commented Mar 29, 2019

Uh oh!

BusyByte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BusyByte commented Mar 30, 2019

Uh oh!

BusyByte commented Apr 13, 2019

Uh oh!

a1shadows commented Apr 14, 2019

Uh oh!

BusyByte left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a1shadows commented Apr 22, 2019

Uh oh!

a1shadows commented May 1, 2019

Uh oh!

BusyByte commented May 5, 2019

Uh oh!

BusyByte May 5, 2019

Choose a reason for hiding this comment

Uh oh!

a1shadows Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

BusyByte May 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BusyByte May 5, 2019

Choose a reason for hiding this comment

Uh oh!

BusyByte May 5, 2019

Choose a reason for hiding this comment

Uh oh!

BusyByte commented May 5, 2019

Uh oh!

BusyByte commented May 5, 2019

Uh oh!

a1shadows commented Jun 19, 2019

Uh oh!

a1shadows commented Jun 19, 2019

Uh oh!

BusyByte commented Jun 22, 2019

Uh oh!

BusyByte commented Jun 22, 2019

Uh oh!

BusyByte commented Jun 29, 2019

Uh oh!

a1shadows commented Jul 1, 2019 via email

Uh oh!

a1shadows commented Jul 15, 2019

Uh oh!

BusyByte commented Jul 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BusyByte commented Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a1shadows commented Jan 15, 2020 via email

Uh oh!

a1shadows commented Jan 19, 2020

Uh oh!

BusyByte commented Jan 22, 2020

Uh oh!

a1shadows commented Jan 26, 2020

BusyByte left a comment •

edited

Loading

BusyByte May 5, 2019 •

edited

Loading

BusyByte commented Jul 20, 2019 •

edited

Loading

BusyByte commented Jan 15, 2020 •

edited

Loading