smaikkeli / tiralabra

Return and tracking for the course Data Structures and Algorithms Lab
0 stars 0 forks source link

Code review - Schmaigul.4.connect #1

Open tammekasra opened 1 year ago

tammekasra commented 1 year ago

Vertaisarviointi/Peer review

The game play of the program

I tried to open the file and run the program as it was shown within the instructions, albeit it sadly did not work for some reason. I uploaded the video here demonstrating what happened - https://youtu.be/Gfa3w2BcuS0 I did use the python AIConnectFour.py command line etc, but it gives me the following error -> Traceback (most recent call last): .... TypeError: integer argument expected, got float.

I also tried in my own connect 4 program to apply the pygame user interface (UI) and most of the time me myself also had the same issues constantly.

Testing of the algorithm

The testing is really nicely done! The fact that you have generated situations (game positions ) where the A.I has to find the winning move, a drawing move and not a loosing move. You demonstrated clearly that it takes more time to generate an efficient move depending of the depth of the minimax algorithm - the bigger the depth, the longer it takes (obviously it then tries to find the best move). What is interesting is that you got the results in a exponential fashion, which is the actual reality of an any given program when it needs to analyze further (deeper). One thing I do recommend doing is that the A.I plays against itself and runs for a while or so, to see if the algorithm itself is perfect enough that the game always ends up in a draw or not ( in my algorithm it sadly isnt a constant draw.... ) I would have really wanted to put my own testing algorithm to use it on your program, but whenever I run either test_minimax.py or testing.py, it gives me an constant error that It cannot find the utils.ai_logic for what ever reason.... I wanted to implement my own testing here to see how it your algorithm would play against itself, but I sadly for the previous reason can not do that....

MiniMax algorithm

The MiniMax algorithm is nicely and clearly represented. I really like the idea of the function of iterative_deepening that tries to put the all possible next step positions in a order starting from the most winning situation to the least and use that iteration to make the data pruning even faster. Albeit, I am not 100% sure, since the minimax algorithm despite finding the best solution first using the iterative deepening, the minimax algorithm still goes through all the other possibilities, despite finding the best solution first. (The recursion functions wont stop if one ideal solution is already found). This is how I understood how the minimax algorithm works, I do recommend and it would be really interesting if you could do the minimax pruning performance test with and without the iterative_deepening to see if there is a clear difference or not.

Furthermore there is a function called "block use" that tries to block the the opposing players winning move. I do like the idea, albeit I think minimax already does it by itself in evauluate_position? The problem at least for my program (which I have tested also), is that if the minimax algorithm has constantly for what ever reason a winning or a drawing positions and it suddenly sees that the opponent is winning, then the A.I despite blocking it (block use function), after that it start to evaluate the situation from the beginning. What I mean by this is that, in each step, the A.I has to (at least I think so) have a stable evaluation - first moves evaluation is for example -3, then the second move is -1,5, then the third is 0, then it is +4 at some point. (The first moves evaluation has to have an impact on the second move and the evaluations has to either increase or be constantly stable, not jumping up and down - which the block use might do.... ). I might be completly wrong with these statements, but this is how I interpreted my own 4 connect.... (it would be nice to have a discussion about it in the future....)

The quality of the coding

The code itself is really simple to understand and is clearly mentioned what happened in each step. I also want to say thank you that me myself who has the same project of connect 4, I never have had done any testing before in my life, and your testing is something worthwhile to learn from and easy to remember for future practise.

I honestly don't know what to write more, but I do appreciate your work! Well done! (I got some ideas to implement on my own program and I've learned a lot) (If it is some how possible to show how to initiate the program somehow, please send me a message or something, I would really love to try if you algorithm plays against mine to see what happens)

smaikkeli commented 1 year ago

Hi, thanks for the feedback, I got a lot from that! I think I fixed the bug. Would you like to try to clone the project and run it again? The reason for blocking the winning move is exactly what you were explaining. I was not able to implement immidiate blocking in the algorithm itself, so I made a workaround of checking it with its own function.

smaikkeli commented 1 year ago

Also, testing is done with the command _python -m unittest -v test.testminimax and python -m test.testing

tammekasra commented 1 year ago

Yes! I will try to do it soon in around 30 minutes (at 18:00). P.S Sorry if I wrote little bit too much in the feedback and if it might have been little bit confusing....

tammekasra commented 1 year ago

Ok, I tested the program and now it works like a charm! Both the UI and both of the tests!