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

katago: init at 1.3.5 #82082

Merged
merged 1 commit into from Mar 31, 2020
Merged

katago: init at 1.3.5 #82082

merged 1 commit into from Mar 31, 2020

Conversation

OmnipotentEntity
Copy link
Contributor

Motivation for this change

Katago is the current strongest open source go AI engine. This derivation provides both the OpenCL (default) and CUDA versions of the engine.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@OmnipotentEntity
Copy link
Contributor Author

The specific versions of cudnn and cudatoolkit is because they have to match, CUDA performance is actually quite bad reportedly for 9.2 and KataGo, so I selected 10.0, which is the version that the author mentioned he was originally running when he wrote this program. This program also is tested as working on cuda 10.2, but it requires using a version of cudnn that has not been accepted into the nixpkgs repository yet.

@OmnipotentEntity
Copy link
Contributor Author

@GrahamcOfBorg build katago katagoWithCuda
@GrahamcOfBorg eval "p: (p {}).katago.override { cudaSupport = true; useTcmalloc = true; }"

Please and thank you.

@OmnipotentEntity
Copy link
Contributor Author

Updated default CUDA Toolkit from 10.0 to 10.1 because that's what the repository readme suggested.

@OmnipotentEntity
Copy link
Contributor Author

@rasendubi @erikarvstedt @averelld Hi there, you were active on the original pull request for Leela Zero. If it's not too much trouble, can one of you review this pull request?

@averelld
Copy link
Contributor

Great that this is finally being packaged!
I've built an tested several cuda/opencl * tcmalloc combos successfully (builds and runs fine).

I think just defaulting to tcmalloc would be better if we even make it configurable.

Also, the buildbot probably won't evaluate anything with cuda because of it's unfree nature.

Other than the above, LGTM, the quicker we merge the better!

@OmnipotentEntity OmnipotentEntity force-pushed the katago-init branch 4 times, most recently from 919b994 to e6c9040 Compare March 25, 2020 00:45
@OmnipotentEntity
Copy link
Contributor Author

OmnipotentEntity commented Mar 25, 2020

Ugh, I need to be more careful about screwing around with files locally when they still might change later. This should work now.

@GrahamcOfBorg build katago katagoWithCuda

@OmnipotentEntity OmnipotentEntity force-pushed the katago-init branch 2 times, most recently from 8ab5936 to 67f8f4d Compare March 29, 2020 21:03
@OmnipotentEntity OmnipotentEntity changed the title katago: init at 1.3.3 katago: init at 1.3.4 Mar 29, 2020
@OmnipotentEntity
Copy link
Contributor Author

OmnipotentEntity commented Mar 29, 2020

New release, packaged that instead of the old release.

EDIT: Author updated the tag, so the latest force push is updating the sha commit to match the new tag.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/137

@OmnipotentEntity
Copy link
Contributor Author

@GrahamcOfBorg build katago katagoWithCuda

Once again into the night we go.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Mar 30, 2020

You may have to restrict the platforms: it seems to require SSE, which rules out i686 and aarch64.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Mar 30, 2020

Or is there something more specific?

No, I don't think there is anything more specific.
If it's cross-platform try using platforms.x86_64, otherwise restrict it to linux only with "x86_64-linux".

@OmnipotentEntity
Copy link
Contributor Author

@GrahamcOfBorg build katago katagoWithCuda

Copy link
Contributor

@rnhmjoj rnhmjoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression looks good to me and ofBorg builds it.
I don't have the hardware to try, though, maybe only openCL.
I'd wait a bit to give @averelld the possibility to finish the review.

@averelld
Copy link
Contributor

I just rebuilt with v1.3.4, with and without cuda still works great.

@OmnipotentEntity OmnipotentEntity changed the title katago: init at 1.3.4 katago: init at 1.3.5 Mar 31, 2020
@OmnipotentEntity
Copy link
Contributor Author

He released another minor version to fix a few newly discovered bugs. Sorry about the second version jump in this init pull request.

@averelld
Copy link
Contributor

It's a super short build, I like being up to date (and still works for me).

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Mar 31, 2020

All right, let's merge this. Thank you @OmnipotentEntity!

@rnhmjoj rnhmjoj merged commit 9cc3372 into NixOS:master Mar 31, 2020
@OmnipotentEntity
Copy link
Contributor Author

Thanks for your help @averelld @rnhmjoj <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants