LeonardoTredese / s302294-computational-intelligence-2022-2023

0 stars 0 forks source link

Lab 3 - Code Review #5

Open FabioSofer opened 1 year ago

FabioSofer commented 1 year ago

Overview

A first simple critique is that the code is left without comments in the parts regarding min-max and RL, as such the reader is offered no help understanding the trickier lines in the program.

Comments by file

Game.py

The inclusion of a separate class handling matches is very clean and efficient, while there is a sizable overlap between Nim and NimNode which makes me think they should be rolled into a single, hashable Nim class with all the necessary methods.

Players.py & evolved_agents.py

Good to see a variety of different players, in particular, Hybrid Ply is a very interesting agent mixing hardcoded and evolutionary rules, other than that, the evoled agents file could use more comments for improved readability, but otherwise is a unique approach to the problem(population + lexicase selection).

Minmax.py

The minmax approach works, as it is a competent beam search leveraging file caches but there seems to be no pruning done to make the searching more efficient, limiting the potential of the solution.

RL.py

It is a clean and efficient solution for the reinforcement learning approach to Nim, the Trainer class makes is it easy and functional to train against different opponents and get the resulting trained agent.

Conclusions

Overall this an extremely well executed lab in my opinion, with only very small code fixes to be proposed along side wider commenting of the functions and their interior.

LeonardoTredese commented 1 year ago

Hello,

First of all thank you for your review. I am sorry about the comments, I will try to add them at some point to increase readablilty.

I agree on the redundancies in the definition of both NimNode and Nim, but unfortunately I need both since my adaptive players are thought for a game that has a fixed number of rows it would break them & min-max and RL are more efficient with variable number of rows.

I disagree on min-max pruning. I exploit the fact that the only values that the function can take are {-1, +1}. for example as soon as I find +1 and I am Max than I do not need o explore any further ply, hence pruning the recursive call tree.

defaultdict at the first access of any key it initializes the corresponding value with the function specified in the constructor. I use it to avoid computing the whole table at the beginning and to do it just in time.

I'm sorry again for the commenting I fix this situation as soon as I can.

FabioSofer commented 1 year ago

I misunderstood how defaultdict worked I am sorry, I edited my review as such.