Skip to content

Add missing match on onnx/model.onnx download #472

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 26, 2025
Merged

Add missing match on onnx/model.onnx download #472

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf19a02
Add missing `match` on `onnx/model.onnx` download
alvarobartt Jan 16, 2025
bc17257
Merge branch 'main' into add-missing-onnx-handling
alvarobartt Feb 26, 2025
855dc50
Fix handling of `download_onnx` function output
alvarobartt Feb 26, 2025
297944a
Capture `model_files` and check if empty
alvarobartt Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions backends/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,17 @@ async fn init_backend(
{
if let Some(api_repo) = api_repo.as_ref() {
let start = std::time::Instant::now();
download_onnx(api_repo)
let model_files = download_onnx(api_repo)
.await
.map_err(|err| BackendError::WeightsNotFound(err.to_string()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you only need to add a ? after the map_err?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because it still succeeds as far as I remember the download_onnx function was not producing errors but just warnings so empty ONNX files was not considered an error and so on it was printing that the files where there while those really weren't + this doesn't need to fail as it needs to fallback to the Safetensors backend if the ONNX files are not there.

Anyway I could've missed something, so please let me know!

tracing::info!("Model ONNX weights downloaded in {:?}", start.elapsed());
.map_err(|err| BackendError::WeightsNotFound(err.to_string()))?;
match model_files.is_empty() {
true => {
tracing::error!("Model ONNX files not found in the repository")
}
false => {
tracing::info!("Model ONNX weights downloaded in {:?}", start.elapsed())
}
}
}

let backend = OrtBackend::new(&model_path, dtype.to_string(), model_type.clone());
Expand Down Expand Up @@ -530,8 +537,11 @@ async fn download_onnx(api: &ApiRepo) -> Result<Vec<PathBuf>, ApiError> {
Err(err) => {
tracing::warn!("Could not download `model.onnx`: {err}");
tracing::info!("Downloading `onnx/model.onnx`");
let p = api.get("onnx/model.onnx").await?;
model_files.push(p.parent().unwrap().to_path_buf())

match api.get("onnx/model.onnx").await {
Ok(p) => model_files.push(p.parent().unwrap().to_path_buf()),
Err(err) => tracing::warn!("Could not download `onnx/model.onnx`: {err}"),
};
Comment on lines -533 to +544
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this virtually the same? How is the error handled in this fn?

}
};

Expand Down
Loading