-
Notifications
You must be signed in to change notification settings - Fork 0
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?
Conversation
@sethaxen please give me access to the main branch. |
@unnir Okay, I've temporarily disabled branch protection rules, but let me know when the bulk of the code has been contributed, so I can re-enable it. |
Great, thanks! |
windows_app/model/model.pth
Outdated
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:
import requests
import hashlib
import logging
import os
def download_artifact(url: str, download_path: str, expected_hash: str=None, hash_algorithm: str='sha256', chunk_size: int=2**20):
"""
Downloads an artifact from a given URL and optionally checks its hash.
Parameters
----------
url : str
The URL of the artifact.
download_path : str
The path where the artifact will be downloaded.
expected_hash : str, optional
The expected hash of the artifact. (default: `None`)
hash_algorithm : str, optional
The hash algorithm to use. (default: 'sha256')
chunk_size : int, optional
The size of the chunks to use when downloading the artifact. (default: 2**20)
Returns
-------
None
Raises:
- ValueError: If the computed hash does not match the expected hash.
"""
response = requests.get(url, stream=True)
response.raise_for_status() # Check for any errors and raise an exception if found.
hasher = hashlib.new(hash_algorithm)
with open(download_path, 'wb') as file:
for chunk in response.iter_content(chunk_size=chunk_size):
file.write(chunk)
hasher.update(chunk)
computed_hash = hasher.hexdigest()
if expected_hash:
if computed_hash != expected_hash:
os.remove(download_path) # Delete the downloaded file if the hash does not match.
raise ValueError(f'Hash mismatch: expected {expected_hash}, but got {computed_hash}.')
else:
logging.debug(f'Computed hash: {computed_hash}')
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.
We prefer the weights be stored separately from the code.
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.
We prefer the weights be stored separately from the code.
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.
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 a model.path
. If the weights are not part of the package, then what is this model.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?
The idea of downloading the weights is cool, but what if a user has no internet connection?
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?
Thanks, can you also give the PR a more descriptive name? I'll look through the code more thoroughly shortly. |
The update includes:
Windows Application. Complete code for the Windows application. Compile it using
pyInstaller
(https://pyinstaller.org/en/stable/). Check thewindows_app
folder for more details.1D CNN Model Training. Full source code for training the 1D CNN model, runnable on Google Colab or locally. Refer to the Colab link in the README or directly access the Jupyter Notebook here.