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

[multimodal] Unifying dump/export model by introducing ExportMixin #2840

Merged
merged 4 commits into from
Feb 7, 2023

Conversation

liangfu
Copy link
Collaborator

@liangfu liangfu commented Feb 4, 2023

Issue #, if available:

Description of changes:

  1. Renamed unittest files for dump/export models
    multimodal/tests/unittests/others/{test_deployment.py => test_deployment_onnx.py}
    multimodal/tests/unittests/{others_2/test_predictor_model_dump.py => others/test_deployment_torch.py}
  2. Introduced ExportMixin for MultiModalPredictor
    This would help reduce the size of the predictor.py file.
    The dump_model and export_onnx related functions are moved to the ExportMixin class

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@liangfu liangfu added the model list checked You have updated the model list after modifying multimodal unit tests/docs label Feb 4, 2023
@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Job PR-2840-d9d22dd is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-2840/d9d22dd/index.html

@github-actions
Copy link

github-actions bot commented Feb 7, 2023

Job PR-2840-bc30e43 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-2840/bc30e43/index.html

from .onnx import get_onnx_input


class ExportMixin:
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this Mixin also supports matcher?

Copy link
Collaborator Author

@liangfu liangfu Feb 7, 2023

Choose a reason for hiding this comment

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

I'm not quite sure whether it would support matcher. It should behavior exactly as before. This is no implementation change. Just moved dump_model and export_onnx out of the MultiModalPredictor.

Copy link
Contributor

@suzhoum suzhoum left a comment

Choose a reason for hiding this comment

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

Thanks for the refactor!

@github-actions
Copy link

github-actions bot commented Feb 7, 2023

Job PR-2840-a6df52b is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-2840/a6df52b/index.html

@liangfu liangfu merged commit d0513c0 into autogluon:master Feb 7, 2023
@liangfu liangfu deleted the export-mixin-1 branch February 7, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
model list checked You have updated the model list after modifying multimodal unit tests/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants