Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use get_input_channels() rather than MAX_INPUT_CHANNELS when populating network input array. #577

Merged
merged 3 commits into from
May 11, 2018

Conversation

Tilps
Copy link
Contributor

@Tilps Tilps commented May 10, 2018

(This is just a quick implementation of the fix KillerDucky proposed in #576)

@Tilps Tilps changed the base branch from master to next May 10, 2018 23:42
@Tilps Tilps changed the title Use get_input_planes() rather than MAX_INPUT_PLANES when populating network input array. Use get_input_channels() rather than MAX_INPUT_CHANNELS when populating network input array. May 10, 2018
@killerducky
Copy link
Collaborator

I was doing one too but let's use your. How about some comments like this:

+    // NNPlanes is a static structure that has room for the maximum
+    // number of planes (V1). So here use MAX_INPUT_CHANNELS.
+    // (It might be nice to make it dynamic or some other solution.)
+    //
+    // But the rest of the code should use get_input_channels()
+    // to find how many we are actually using.

@Tilps
Copy link
Contributor Author

Tilps commented May 11, 2018

I added a comment.
'Very early, very quick tests' suggest this fix is at least +100Elo in self play at training type visit counts.

@dubslow
Copy link
Contributor

dubslow commented May 11, 2018

I think our next priority needs to be a lot more testing code, that something like this went unnoticed for so long is not a good sign for the health of the codebase

@Tilps
Copy link
Contributor Author

Tilps commented May 11, 2018

Testing still positive Elo, but down from previous highs. +40Elo after 35 games. I am using net 245, since I had it handy and thought that overtraining might obfuscate the win.

@killerducky killerducky merged commit 8d7709f into glinscott:next May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants