-
Notifications
You must be signed in to change notification settings - Fork 57
Added Logic to Download Model from HugginFace #917
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
Added Logic to Download Model from HugginFace #917
Conversation
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.
added minor comments, rest looks good!
ads/aqua/model/model.py
Outdated
f"The model path {os_path} does not container either {ModelFormat.SAFETENSORS} " | ||
f"or {ModelFormat.GGUF} files. Please check if the path is correct or the model " | ||
f"The model {model_name} does not contain either {ModelFormat.SAFETENSORS.value} " | ||
f"or {ModelFormat.GGUF.value} files in {os_path} or HugginFace. Please check if the path is correct or the model " |
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.
typo, HugginFace
with Hugging Face repository
.
ads/aqua/model/model.py
Outdated
).siblings | ||
except Exception as e: | ||
huggingface_err_message = str(e) | ||
raise Exception( |
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.
replace Exception with AquaRuntimeError so that it doesn't trigger error 500?
I'm thinking if we should raise exception here or let user proceed with a warning. Fine to keep exception now, we can discuss in team sync regarding this.
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 the same code as the previous version
ads/aqua/model/model.py
Outdated
os.makedirs(local_dir, exist_ok=True) | ||
# Copy the model from the cache to destination | ||
snapshot_download( | ||
repo_id=model_name, local_dir=local_dir, local_dir_use_symlinks=False |
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.
local_dir_use_symlinks
has been deprecated in 0.23.4
which is being used for this iteration. We should remove this parameter otherwise it shows unnecessary deprecation logs to the user when this runs.
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.
Same from the previous version. Let me remove it.
Added Logic to Download Model from HugginFace
download_from_hf: Optional[bool] = True
andlocal_dir: Optional[str] = None
toImportModelDetails
Notebook