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

add automodel structure for unified model loading #469

Merged
merged 2 commits into from
May 28, 2022
Merged

add automodel structure for unified model loading #469

merged 2 commits into from
May 28, 2022

Conversation

fcakyon
Copy link
Collaborator

@fcakyon fcakyon commented May 28, 2022

New AutoDetectionModel class makes it easier to load any yolov5, mmdet, detectron2 model!

from sahi import AutoDetectionModel

# load yolov5 models
detection_model = AutoDetectionModel.from_local(
    model_type='yolov5',
    model_path='yolov5s.pt',
    image_size=640,
)

# load mmdet models
detection_model = AutoDetectionModel.from_local(
    model_type='mmdet',
    model_path='model.pt',
    model_config='config.py',
    image_size=640,
)

# load detectron2 models
detection_model = AutoDetectionModel.from_local(
    model_type='detectron2',
    model_path='model.pt',
    model_config='config.yaml',
    image_size=640,
)

@fcakyon fcakyon added the enhancement New feature or request label May 28, 2022
@fcakyon fcakyon merged commit 02949de into main May 28, 2022
@fcakyon fcakyon deleted the auto-model branch May 28, 2022 21:19
mecevit added a commit to mecevit/sahi that referenced this pull request May 29, 2022
add automodel structure for unified model loading (obss#469)
@@ -540,7 +535,7 @@ def predict(
tqdm.write("Prediction time is: {:.2f} ms".format(prediction_result.durations_in_seconds["prediction"] * 1000))

if dataset_json_path:
if source_is_video == True:
if source_is_video is True:

Choose a reason for hiding this comment

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

Just using if source_is_video is enough


from sahi.utils.file import import_model_class

MODEL_TYPE_TO_MODEL_CLASS_NAME = {

Choose a reason for hiding this comment

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

I think this registry should be better as class variable inside AutoDetectionModel. Also, why don't save the classes directly instead of saving only their name?

Copy link
Collaborator Author

@fcakyon fcakyon May 29, 2022

Choose a reason for hiding this comment

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

yes better move it inside the class.

saved only the names to avoid adding a new module import once a new detector model support is added. this way we only need to update a list of string (instead of adding module import).

can switch to saving directly the classes if its a better practice.

Copy link

@cemilcengiz cemilcengiz May 29, 2022

Choose a reason for hiding this comment

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

Why don't you want to avoid imports? Are they too expensive (I doubt it) or some of them require installation of other packages? As for the better practice, this way one might miss some bugs e.g. incorrectly written class names would not be noticed until the interpreter tries to instantiate them during run time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants