Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Update README.md #1
base: main
Are you sure you want to change the base?
Update README.md #1
Changes from 3 commits
6670112
1c37fb1
0499d77
ca7e99d
14ced61
179f7f9
07a6612
4b8ecff
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is quite large, and It's probably better for us to distribute this in a different manner than bundled with the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up to you guys. In general, this size is considered very small.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but it's also about hygiene. We prefer the weights be stored separately from the code. The idea is that at runtime, the user can provide the path to the weights. If they don't, then the code should check if the weights are present in a predefined location in the package, and if they're not, then they are automatically downloaded, using a method like this:
As for where to place the weights, a simple approach is to make a release in this repo, and add the weights as an artifact to the release. Then the URL of the artifact and the hash can be specified in the code in the next commit.
In terms of how to do this, I suggest you send us the weights now through another means (e-mail, Google Drive, etc), and I can take care of setting up the artifact. For code, I only ask for now that you add placeholder code to automatically download the weights to the right place once we have the URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already the case; the weights are not part of the package. A user has to select weights from the drive, so we do not need to recompile the app every time. This way, I can just send Sam new updated weights, and she can still use the app.
The idea of downloading the weights is cool, but what if a user has no internet connection?
Anyway, currently, I do not have access to a Windows machine, so I won't be able to test the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you tell me a little more about this workflow then? I see there's a
windows_app/model/weights.path
documented in the readme, but no such file in the repo, but there is amodel.path
. If the weights are not part of the package, then what is thismodel.path
file, and why is it included? When the user launches the app, must they have already downloaded weights from the drive, or is there then an interface to do that? I also don't have a windows machine to test it for myself.RE the windows app, how keyed to windows is it? Is there a way it could be run on other platforms?
Then if weights are unavailable, an error would be thrown instructing the user how to get the weights. How would this be less of an issue if the weights are stored in Google Drive though?