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

Check if nnuenet file exists before downloading #4253

Conversation

shtayerc
Copy link

@shtayerc shtayerc commented Dec 5, 2022

Hello, after this commit Makefile always downloads net file.
This PR checks if it already exists before downloading. That behavior was present in previous release

@MinetaS
Copy link
Contributor

MinetaS commented Dec 6, 2022

I believe we already skip the download phase when the default network exists.

if test -f "$(nnuenet)"; then \

> make -j32 profile-build ARCH=x86-64-vnni512 COMP=clang
Default net: nn-ad9b42354671.nnue
nn-ad9b42354671.nnue available.
Network validated

Please check if your network file is up to date & not corrupted.

@shtayerc
Copy link
Author

shtayerc commented Dec 6, 2022

@MinetaS file exist check is after check for curl and wget. Which requires curl and wget even if it is not used. I want to avoid additional dependencies if possible.

@vondele
Copy link
Member

vondele commented Dec 7, 2022

It probably needs some more care, we might still want to check that a net that is available has the right sha, and stop (or download) if not. I don't think we should by default assume a net that is present is good (could be a leftover from an aborted download).

@shtayerc
Copy link
Author

shtayerc commented Dec 8, 2022

Understandable. I modified code to check sha for local file.

@vondele
Copy link
Member

vondele commented Dec 8, 2022

so, the idea is not to require wget or curl if they are not going to be used. This makes sense. However, I'll need to have a more careful look, this patch doesn't look quite right.

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.

None yet

3 participants