MiniGo style pass and and minor logic enhancements. (#131)

Logic changes:
* In LibertyTracker.swift, use dictionary indices to avoid force unwrapping. Removed a lot of bangs!
* Make `ModelConfiguration.boardSize` public since the initializer takes a `boardSize` for better transparency. It's a little weird for an initializer to take a constant, store it and make it private.
* Reduce nesting in `LibertyTracker.updateLibertiesAfterRemovingCapturedStones(_:)` with `compactMap(_:)`.
* Simplify `assignNewGroupID()` with a `defer` block.

Formatting changes:
* Make every file end with exactly 1 empty line.
* Add some minor comments.
* Remove empty lines between the start of a type declaration or extension and the first declaration for consistency.
* Make `GoModel` code fit within 100 columns.

Author: rxwei

MiniGo/GameLib/Board.swift

@@ -46,7 +46,6 @@ struct Board: Hashable {
 }
 
 extension Board: CustomStringConvertible {
-
     var description: String {
         var output = ""
 
@@ -103,4 +102,3 @@ extension Board: CustomStringConvertible {
         return output
     }
 }
-

MiniGo/GameLib/BoardState.swift

@@ -38,8 +38,9 @@ private enum PositionStatus: Equatable {
 /// `BoardState` checks whether a new placed stone is legal or not. If so,
 /// creates a new snapshot.
 public struct BoardState {
-
+    /// The game configuration.
     let gameConfiguration: GameConfiguration
+    /// The color of the next player.
     let nextPlayerColor: Color
 
     /// The position of potential `ko`. See `IllegalMove.ko` for details.
@@ -218,7 +219,6 @@ extension BoardState: Equatable {
 }
 
 extension Board {
-
     /// Calculates all legal moves on board.
     fileprivate func allLegalMoves(
         ko: Position?,
@@ -276,8 +276,7 @@ extension Board {
 
         for neighbor in position.neighbors(boardSize: self.size) {
             guard let group = libertyTracker.group(at: neighbor) else {
-                // If the neighbor is not occupied, no liberty group, the position is
-                // OK.
+                // If the neighbor is not occupied, no liberty group, the position is OK.
                 return false
             }
             if group.color == nextPlayerColor {
@@ -288,14 +287,14 @@ extension Board {
             }
         }
 
-        // After removing the new postion from liberties, if there is no liberties
-        // left, this move is suicide.
+        // After removing the new postion from liberties, if there is no liberties left, this move
+        // is suicide.
         possibleLiberties.remove(position)
         return possibleLiberties.isEmpty
     }
 
-    /// Checks whether the position is a potential ko, i.e., whether the position is surrounded by all
-    /// sides belonging to the opponent.
+    /// Checks whether the position is a potential ko, i.e., whether the position is surrounded by
+    /// all sides belonging to the opponent.
     ///
     /// This is an approximated algorithm to find `ko`. See https://en.wikipedia.org/wiki/Ko_fight
     /// for details.
@@ -320,7 +319,6 @@ extension Board {
     /// `komi` is the points added to the score of the player with the white stones as compensation
     /// for playing second.
     fileprivate func scoreForBlackPlayer(komi: Float) -> Float {
-
         // Makes a copy as we will modify it over time.
         var scoreBoard = self
 
@@ -335,8 +333,9 @@ extension Board {
             }
         }
 
-        // Second pass: Calculates the territory and borders for each empty position, if there is any.
-        // If territory is surrounded by the stones in same color, fills that color in territory.
+        // Second pass: Calculates the territory and borders for each empty position, if there is
+        // any. If territory is surrounded by the stones in same color, fills that color in
+        // territory.
         while !emptyPositions.isEmpty {
             let emptyPosition = emptyPositions.removeFirst()
 
@@ -402,7 +401,8 @@ extension Board {
             for neighbor in currentPosition.neighbors(boardSize: self.size) {
                 if self.color(at: neighbor) == nil {
                     if !territory.contains(neighbor) {
-                        // We have not explored this (empty) position, so queue it up for processing.
+                        // We have not explored this (empty) position, so queue it up for
+                        // processing.
                         candidates.insert(neighbor)
                     }
                 } else {
@@ -412,12 +412,10 @@ extension Board {
             }
         } while !candidates.isEmpty
 
-        precondition(
-            territory.allSatisfy { self.color(at: $0) == nil },
-            "territory must be all empty (no stones).")
-        precondition(
-            borders.allSatisfy { self.color(at: $0) != nil },
-            "borders cannot have empty positions.")
+        precondition(territory.allSatisfy { self.color(at: $0) == nil },
+                     "territory must be all empty (no stones).")
+        precondition(borders.allSatisfy { self.color(at: $0) != nil },
+                     "borders cannot have empty positions.")
         return (territory, borders)
     }
 }

MiniGo/GameLib/Color.swift

@@ -28,4 +28,3 @@ extension Color: CustomStringConvertible {
         }
     }
 }
-

MiniGo/GameLib/GameConfiguration.swift

@@ -15,19 +15,19 @@
 /// Represents an (immutable) configuration of a Go game.
 public struct GameConfiguration {
     /// The board size of the game.
-    let size: Int
+    public let size: Int
 
     /// The points added to the score of the player with the white stones as compensation for playing
     /// second.
-    let komi: Float
+    public let komi: Float
 
     /// The maximum number of board states to track.
     ///
     /// This does not include the the current board.
-    let maxHistoryCount: Int
+    public let maxHistoryCount: Int
 
     /// If true, enables debugging information.
-    let isVerboseDebuggingEnabled: Bool
+    public let isVerboseDebuggingEnabled: Bool
 
     public init(
         size: Int,
@@ -41,4 +41,3 @@ public struct GameConfiguration {
         self.isVerboseDebuggingEnabled = isVerboseDebuggingEnabled
     }
 }
-

MiniGo/GameLib/LibertyTracker.swift

@@ -33,14 +33,13 @@ struct LibertyGroup {
 /// placing a new stone, we make a copy, update it and then attach it to new
 /// board snapshot to track state.
 struct LibertyTracker {
-
+    /// The game configuration.
     private let gameConfiguration: GameConfiguration
 
-    // Tracks the liberty groups. For a position (stone) having no group,
-    // groupIndex[stone] should be nil. Otherwise, the group ID should be
-    // groupIndex[stone] and its group is groups[groupIndex[stone]].
-    // The invariance check can be done via checkLibertyGroupsInvariance helper
-    // method.
+    // Tracks the liberty groups. For a position (stone) having no group, `groupIndex[stone]` should
+    // be `nil`. Otherwise, the group ID should be `groupIndex[stone]` and its group is
+    // `groups[groupIndex[stone]]`. The invariance check can be done via the
+    // `checkLibertyGroupsInvariance()` helper method.
     private var nextGroupIDToAssign = 0
     private var groupIndex: [[Int?]]
     private var groups: [Int: LibertyGroup] = [:]
@@ -64,9 +63,8 @@ struct LibertyTracker {
     }
 }
 
-/// Extend `LibertyTracker` to have a mutating method by placing a new stone.
+// Extend `LibertyTracker` to have a mutating method by placing a new stone.
 extension LibertyTracker {
-
     /// Adds a new stone to the board and returns all captured stones.
     mutating func addStone(at position: Position, withColor color: Color) throws -> Set<Position> {
         precondition(groupIndex(for: position) == nil)
@@ -81,7 +79,6 @@ extension LibertyTracker {
         var friendlyNeighboringGroupIDs = Set<Int>()
 
         for neighbor in position.neighbors(boardSize: gameConfiguration.size) {
-
             // First, handle the case neighbor has no group.
             guard let neighborGroupID = groupIndex(for: neighbor) else {
                 emptyNeighbors.insert(neighbor)
@@ -184,11 +181,9 @@ extension LibertyTracker {
 
     /// Assigns a new unique group ID.
     mutating private func assignNewGroupID() -> Int {
-        let newID = nextGroupIDToAssign
-        precondition(groups[newID] == nil)
-
-        nextGroupIDToAssign += 1
-        return newID
+        defer { nextGroupIDToAssign += 1 }
+        precondition(!groups.keys.contains(nextGroupIDToAssign))
+        return nextGroupIDToAssign
     }
 
     /// Creates a new group for the single stone with liberties.
@@ -200,7 +195,7 @@ extension LibertyTracker {
         let newID = assignNewGroupID()
         let newGroup = LibertyGroup(id: newID, color: color, stones: [stone], liberties: liberties)
 
-        precondition(groups[newID] == nil)
+        precondition(!groups.keys.contains(newID))
         groups[newID] = newGroup
         groupIndex[stone.x][stone.y] = newID
         assert(checkLibertyGroupsInvariance())
@@ -243,27 +238,27 @@ extension LibertyTracker {
 
     /// Captures the whole group and returns all stones in it.
     mutating private func captureGroup(_ groupID: Int) -> Set<Position> {
-        let deadGroup = groups.removeValue(forKey: groupID)!
+        guard let index = groups.index(forKey: groupID) else {
+            fatalErrorForGroupsInvariance(groupID: groupID)
+        }
+        let deadGroup = groups.remove(at: index).value
         for stone in deadGroup.stones {
             groupIndex[stone.x][stone.y] = nil
         }
         return deadGroup.stones
     }
 
     /// Updates all neighboring groups' liberties.
-    mutating private func updateLibertiesAfterRemovingCapturedStones(_ capturedStones: Set<Position>) {
+    mutating private func updateLibertiesAfterRemovingCapturedStones(
+        _ capturedStones: Set<Position>
+    ) {
         let size = gameConfiguration.size
         for capturedStone in capturedStones {
-            for neighbor in capturedStone.neighbors(boardSize: size) {
-                if let neighborGroupdID = groupIndex(for: neighbor) {
-                    guard groups.keys.contains(neighborGroupdID) else {
-                        fatalErrorForGroupsInvariance(groupID:neighborGroupdID)
-                    }
-                    // This force unwrap is safe as we checked the key existence above. As
-                    // the value in the groups is struct. We need the force unwrap to do
-                    // mutation in place.
-                    groups[neighborGroupdID]!.liberties.insert(capturedStone)
+            for neighborGroupID in capturedStone.neighbors(boardSize: size).compactMap(groupIndex) {
+                guard let index = groups.index(forKey: neighborGroupID) else {
+                    fatalErrorForGroupsInvariance(groupID: neighborGroupID)
                 }
+                groups.values[index].liberties.insert(capturedStone)
             }
         }
         assert(checkLibertyGroupsInvariance())
@@ -277,7 +272,7 @@ extension LibertyTracker {
 
         print(message)
 
-        /// Prints the group index for the board.
+        // Prints the group index for the board.
         let size = gameConfiguration.size
         for x in 0..<size {
             for y in 0..<size {
@@ -298,5 +293,3 @@ extension LibertyTracker {
         }
     }
 }
-
-

MiniGo/GameLib/Position.swift

@@ -14,21 +14,24 @@
 
 /// Represents a position in a Go game.
 public struct Position: Hashable, Equatable {
-    var x: Int
-    var y: Int
+    public var x: Int
+    public var y: Int
+    
+    public init(x: Int, y: Int) {
+        self.x = x
+        self.y = y
+    }
 }
 
 /// Returns all valid neighbors for the given position on board.
 extension Position {
-
     func neighbors(boardSize size: Int) -> [Position] {
         let neighbors = [
             Position(x: x+1, y: y),
             Position(x: x-1, y: y),
             Position(x: x, y: y+1),
             Position(x: x, y: y-1),
         ]
-
         return neighbors.filter { 0..<size ~= $0.x && 0..<size ~= $0.y }
     }
 }

MiniGo/Models/GoModel.swift

@@ -17,17 +17,17 @@
 import TensorFlow
 
 public struct ModelConfiguration {
-    /// size of Go board (typically 9 or 19)
-    let boardSize: Int
-    /// output feature count of conv layers in shared trunk
+    /// The size of the Go board (typically `9` or `19`).
+    public let boardSize: Int
+    /// The number of output features of conv layers in shared trunk.
     let convWidth: Int
-    /// output feature count of conv layer in policy head
+    /// The output feature count of conv layer in policy head.
     let policyConvWidth: Int
-    /// output feature count of conv layer in value head
+    /// The output feature count of conv layer in value head.
     let valueConvWidth: Int
-    /// output feature count of dense layer in value head
+    /// The output feature count of dense layer in value head.
     let valueDenseWidth: Int
-    /// number of layers (typically equal to boardSize)
+    /// The number of layers (typically equal to `boardSize`).
     let layerCount: Int
 
     public init(boardSize: Int) {
@@ -143,32 +143,32 @@ public struct GoModel: Layer {
 
     public init(configuration: ModelConfiguration) {
         self.configuration = configuration
+        
         initialConv = ConvBN(
             filterShape: (3, 3, 17, configuration.convWidth),
             padding: .same,
             bias: false)
-
         residualBlocks = (1...configuration.boardSize).map { _ in
             ResidualIdentityBlock(featureCounts: (configuration.convWidth, configuration.convWidth))
         }
-
         policyConv = ConvBN(
             filterShape: (1, 1, configuration.convWidth, configuration.policyConvWidth),
             padding: .same,
             bias: false,
             affine: false)
         policyDense = Dense<Float>(
-            inputSize: configuration.policyConvWidth * configuration.boardSize * configuration.boardSize,
+            inputSize: configuration.policyConvWidth * configuration.boardSize
+                * configuration.boardSize,
             outputSize: configuration.boardSize * configuration.boardSize + 1,
             activation: {$0})
-
         valueConv = ConvBN(
             filterShape: (1, 1, configuration.convWidth, configuration.valueConvWidth),
             padding: .same,
             bias: false,
             affine: false)
         valueDense1 = Dense<Float>(
-            inputSize: configuration.valueConvWidth * configuration.boardSize * configuration.boardSize,
+            inputSize: configuration.valueConvWidth * configuration.boardSize
+                * configuration.boardSize,
             outputSize: configuration.valueDenseWidth,
             activation: relu)
         valueDense2 = Dense<Float>(
@@ -188,16 +188,14 @@ public struct GoModel: Layer {
 
         let policyConvOutput = relu(policyConv(output))
         let logits = policyDense(policyConvOutput.reshaped(to:
-                [batchSize,
-                 configuration.policyConvWidth * configuration.boardSize * configuration.boardSize
-                ]))
+            [batchSize,
+             configuration.policyConvWidth * configuration.boardSize * configuration.boardSize]))
         let policyOutput = softmax(logits)
 
         let valueConvOutput = relu(valueConv(output))
         let valueHidden = valueDense1(valueConvOutput.reshaped(to:
-                [batchSize,
-                 configuration.valueConvWidth * configuration.boardSize * configuration.boardSize
-                ]))
+            [batchSize,
+             configuration.valueConvWidth * configuration.boardSize * configuration.boardSize]))
         let valueOutput = valueDense2(valueHidden).reshaped(to: [batchSize])
 
         return GoModelOutput(policy: policyOutput, value: valueOutput, logits: logits)