Spec URL: https://dcavalca.fedorapeople.org/review/katago/katago.spec SRPM URL: https://dcavalca.fedorapeople.org/review/katago/katago-1.14.1-1.fc41.src.rpm Description: KataGo is a strong open-source self-play-trained Go engine, with many improvements to accelerate learning. It can predict score and territory, play handicap games reasonably, and handle many board sizes and rules all with the same neural net. Fedora Account System Username: dcavalca
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=116150262
Note: I'm not 100% I got the alternatives stuff right (or even if alternatives is the right thing to use here).
Some early comments. Why are clblast, tclap and sha2 bundled ? could a %check be added ? maybe katago runtests $ rpm -i katogo-opengl $ katago benchmark terminate called after throwing an instance of 'StringError' what(): -model MODELFILENAME.bin.gz was not specified to tell KataGo where to find the neural net model, and default was not found at /usr/bin/default_model.bin.gz Aborted (core dumped) Where is model ? And looking for default in bin/ is not great. katago.x86_64: W: package-with-huge-docs 99% This package is mostly /usr/share/katago/doc, maybe have a doc subpackage. some lint errors katago-opencl.x86_64: I: package-supports-update-alternatives katago-opencl.x86_64: E: non-readable /usr/bin/katago 0 katago-opencl.x86_64: W: non-executable-in-bin /usr/bin/katago 0 katago-opencl.x86_64: W: no-manual-page-for-binary katago-opencl katago-opencl.x86_64: W: no-documentation katago-opencl.x86_64: E: alternative-link-missing /etc/alternatives/katago katago-opencl.x86_64: E: alternative-generic-name-not-symlink /usr/bin/katago
Thanks! > Why are clblast, tclap and sha2 bundled ? clblast is a heavily cut down and modified copy of the datafiles that clblast upstream provides; tclap is a modified copy of a oneoff upstream version; sha2 is a modified copy and it's deeply integrated into the source. > could a %check be added ? > maybe katago runtests The tests don't work without a model > Where is model ? https://katagotraining.org, see the included README.fedora. This changes often and doesn't have a stable URI, so I didn't think it made sense to include it in the package. > And looking for default in bin/ is not great. Agreed, afaict it looks for where the binary is, but the code wasn't super clear. I'll get an upstream issue filed for this. > katago.x86_64: W: package-with-huge-docs 99% > This package is mostly /usr/share/katago/doc, maybe have a doc subpackage. Sure, I can do that. It would mean the main package becomes just empty, which is probably fine. > some lint errors I suspect these are all side effects of alternatives not playing well with rpmlint, but I will double check.
> Agreed, afaict it looks for where the binary is, but the code wasn't super clear. I'll get an upstream issue filed for this. Looks like this is by design: https://github.com/lightvector/KataGo/blob/cf75cc7dc21ee219bea8e60d869c142332c1b57c/cpp/dataio/homedata.cpp#L86
Spec URL: https://dcavalca.fedorapeople.org/review/katago/katago.spec SRPM URL: https://dcavalca.fedorapeople.org/review/katago/katago-1.14.1-1.fc41.src.rpm Changelog: - move docs to a -doc subpackage
When i do # rpm -ihv review-katago/results/katago-1.14.1-1.fc41.x86_64.rpm error: Failed dependencies: katago-backend = 1.14.1-1.fc41 is needed by katago-1.14.1-1.fc41.x86_64 Then i have to figure out which to use 'eigen' or 'opencl' I think it would be easier on the user if the 'eigen' subpackage was collapsed to the main package. The setup assumes the user is going to read the README.fedora. I did not the first time. The nn url in the readme could be improved to https://katagotraining.org/networks/ Could a katago-setup setup script be added to automate the instructions ? Some other nits from the review katago.spec:40: W: unversioned-explicit-provides bundled(clblast) katago.spec:43: W: unversioned-explicit-provides bundled(sha2) Take a guess on the versions. katago.spec:60: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 60) Is there going to be a follow on gui package to go with this engine ?
(In reply to Tom Rix from comment #7) > When i do > > # rpm -ihv review-katago/results/katago-1.14.1-1.fc41.x86_64.rpm > error: Failed dependencies: > katago-backend = 1.14.1-1.fc41 is needed by katago-1.14.1-1.fc41.x86_64 > > Then i have to figure out which to use 'eigen' or 'opencl' > I think it would be easier on the user if the 'eigen' subpackage was > collapsed to the main package. This is a side effect of using rpm -i. If the packages are all in a repo and one does dnf install katago, dnf should automatically pull in the eigen subpackage. > The setup assumes the user is going to read the README.fedora. > I did not the first time. > > The nn url in the readme could be improved to > https://katagotraining.org/networks/ Will fix, thanks > Could a katago-setup setup script be added to automate the instructions ? Not easily, as there isn't a stable URL to the "last good network". We'd have to scrape it from that web page, which doesn't seem worth the trouble. > Some other nits from the review > katago.spec:40: W: unversioned-explicit-provides bundled(clblast) > > katago.spec:43: W: unversioned-explicit-provides bundled(sha2) > > Take a guess on the versions. These don't really _have_ a version unfortunately :( > katago.spec:60: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line > 60) Will fix, thanks. > Is there going to be a follow on gui package to go with this engine ? Yes, I have a half-done package for q5go I'm planning to submit for review.
Spec URL: https://dcavalca.fedorapeople.org/review/katago/katago.spec SRPM URL: https://dcavalca.fedorapeople.org/review/katago/katago-1.14.1-1.fc41.src.rpm Changelog: - update URL in the README - fix tabs
Copr build: https://copr.fedorainfracloud.org/coprs/build/7482070 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2274250-katago/fedora-rawhide-x86_64/07482070-katago/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Thanks! This is enough for me. Appoved.
The Pagure repository was created at https://src.fedoraproject.org/rpms/katago
FEDORA-2024-89b551f20d (katago-1.14.1-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-89b551f20d
FEDORA-2024-89b551f20d (katago-1.14.1-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-a8933eeeee (katago-1.14.1-1.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-a8933eeeee
FEDORA-2024-a8933eeeee has been pushed to the Fedora 40 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-a8933eeeee \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-a8933eeeee See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-a8933eeeee (katago-1.14.1-1.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.