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

Raise error on suffix-less model path #8561

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

democat3457
Copy link
Contributor

@democat3457 democat3457 commented Jul 12, 2022

This PR raises an Exception when the model passed into DetectMultiBackend does not have a suffix.

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Enforced format support validation in YOLOv5 TensorFlow model conversion.

📊 Key Changes

  • Added an error raise condition for unsupported format checks during model conversion in TensorFlow.

🎯 Purpose & Impact

  • Purpose: Ensures that the user is informed if they attempt to use an unsupported export format when converting YOLOv5 models using the TensorFlow wrapper.
  • Impact: Provides clearer error messaging, preventing potential confusion and troubleshooting time for users. It ensures users only work with compatible formats, enhancing overall user experience with the model conversion process. 🛠️🔍

@glenn-jocher
Copy link
Member

@democat3457 the weight suffix is checked here on L520 against the valid ones and an error is thrown if it doesn't match:

yolov5/models/common.py

Lines 516 to 526 in 574ceed

def model_type(p='path/to/model.pt'):
# Return model type from model path, i.e. path='path/to/model.onnx' -> type=onnx
from export import export_formats
suffixes = list(export_formats().Suffix) + ['.xml'] # export suffixes
check_suffix(p, suffixes) # checks
p = Path(p).name # eliminate trailing separators
pt, jit, onnx, xml, engine, coreml, saved_model, pb, tflite, edgetpu, tfjs, xml2 = (s in p for s in suffixes)
xml |= xml2 # *_openvino_model or *.xml
tflite &= not edgetpu # *.tflite
return pt, jit, onnx, xml, engine, coreml, saved_model, pb, tflite, edgetpu, tfjs

So this PR initially looked like a good idea but I think it's redundant with the above check.

@democat3457
Copy link
Contributor Author

I see, it seems like the case I had was that the string I passed in had no suffix, so the assert wasn't even being checked - I'll update the check suffix method instead

@democat3457 democat3457 changed the title Raise error on invalid model Raise error on suffix-less model path Jul 12, 2022
@glenn-jocher
Copy link
Member

@democat3457 ah yes I just remembered! The check_suffix() method has no suffix as one of the acceptable options in case the user passes a directory, like for OpenVINO models. So I think your original PR is probably good. Can you revert the last commit and I'll go ahead and merge? Thanks!

@democat3457
Copy link
Contributor Author

oh oops, ok then

@glenn-jocher
Copy link
Member

@democat3457 PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

@democat3457 democat3457 deleted the patch-2 branch July 13, 2022 16:46
ctjanuhowski pushed a commit to ctjanuhowski/yolov5 that referenced this pull request Sep 8, 2022
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.

2 participants