MiniGo style pass and and minor logic enhancements. #131
Conversation
- Rename `InferenceModel`'s `prediction(input:)` to `prediction(for:)`. - Rename `GoModel._vjpApplied(to:)` to `GoModel._vjpCall(_:)`. - Minor formatting changes.
|
Thanks, merging now. @xiejw, let me know if you have any comments! |
| 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).") |
There was a problem hiding this comment.
nit: I am personally OK with this style. But Google swift style guide does not allow this (I maybe misread the guide).
There was a problem hiding this comment.
I think it's a very reasonable style, and I don't think the Google Swift Style Guide explicitly bans this (https://google.github.io/swift/#function-calls).
| @@ -51,7 +48,8 @@ let mctsConfiguration = MCTSConfiguration( | |||
| try playOneGame( | |||
| gameConfiguration: gameConfiguration, | |||
| participants: [ | |||
| MCTSPolicy(participantName: "black", predictor: predictor, configuration: mctsConfiguration), | |||
| MCTSPolicy(participantName: "white", predictor: predictor, configuration: mctsConfiguration), | |||
| MCTSPolicy(participantName: "black", predictor: predictor, | |||
There was a problem hiding this comment.
is one argument per line better and more readable?
There was a problem hiding this comment.
I was mainly making things fit within 100 columns, so configuration: got wrapped. Since the code is pretty compact, one argument per line could be better but probably won't make a huge difference.
| // 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) { |
There was a problem hiding this comment.
just curious: groupIndex is a function expecting argument "for:". But compactMap expects a transform with argument "_", right? How does that work?
There was a problem hiding this comment.
Argument labels are not formally part of a function's type signature. This is the result of SE-0111.
|
Thanks @xiejw for reviewing! |
Broadcasting does not support 8-D tensors. Copy Keras implementation: simulating broadcasting via splitting, duplication, and concatenation. Resolves [TF-525](https://bugs.swift.org/browse/TF-525).
Logic changes:
ModelConfiguration.boardSizepublic since the initializer takes aboardSizefor better transparency. It's a little weird for an initializer to take a constant, store it and make it private.LibertyTracker.updateLibertiesAfterRemovingCapturedStones(_:)withcompactMap(_:).assignNewGroupID()with adeferblock.Formatting changes:
GoModelcode fit within 100 columns.