aqtech-ca / mctreesearch4j

MIT License
7 stars 1 forks source link

Review Issue, @TimKam #52

Open TimKam opened 3 years ago

TimKam commented 3 years ago

Dear all,

let us use this issue to discuss my review (you can branch off smaller issues if you prefer). You find the initial review below.

Best, Timotheus

The contribution introduces a Monte Carlo Tree Seach library, implemented in Kotlin, i.e. made available as a JVM library.

  1. Software license: The software is MIT licensed, which is okay.

  2. Contribution and authorship: Both authors have contributed the software.

  3. Scholarly effort: The library itself is rather small (< 1000 LoC in its core). This is, in itself, not a problem. However, the library merely implements some well-known algorithms (whose implementation, may for example, be an assignment in a Master-level course). One can, for sure, argue that the scholarly effort is limited. There seem to be no similar well-documented Java/Kotlin libraries for Monte Carlo Tree Search, so one could imagine that the library provides some value to the broad AI community, i.e. there is probably a marginal benefit. All in all, it would at least be good to see an outline of future potential for this project and some more long-term activity. At the moment, it looks a bit like a one-shot endeavor.

  4. Documentation: The library is documented with examples, has a statement of need ("Objective"), and is installable as a gradle dependency. It would be good to provide gradle tasks that execute the examples. However, there is no API documentation and the code is poorly documented. The code should be properly documented; then, the API documentation can be auto-generated. No community guidelines are provided. This can be easily fixed.

  5. Tests: Tests are only implemented on the examples (not on the library itself), and no CI is set up. It would be good to set up a CI using GitHub actions or similar, and to write unit tests.

  6. Functionality: The library seems to work. However, there seems to be an issue with a dependency of the example project. My build failed because of implementation( "de.magoeke.kotlin:connectfour:1.0-SNAPSHOT" ) and I removed the dependency to build and run the example (and hence the "connect four" example did not work).

Some details, paper: