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

Support image bytearray in AutoMM #2490

Merged
merged 9 commits into from
Dec 1, 2022
Merged

Conversation

suzhoum
Copy link
Contributor

@suzhoum suzhoum commented Nov 28, 2022

Issue #, if available:

Description of changes:
This PR adds support for image bytearrays in AutoMM.
The DataFrame column can be either bytearray or list[bytearray].

Benchmarking on the original image_path workflow with dataset on SageMaker endpoint

test data:
100 x bytearray-encoded images

data preprocess:
0.5s to decode and save to disk to generate image_path to pass in MultiModalPredictor.fit()

predict_proba:
0.1s

With the support of bytearray, we can save the time spent on data preprocessing, which majority of the time is on saving the image to disk.

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

@suzhoum suzhoum changed the title Add bytearray Support image bytearray in AutoMM Nov 28, 2022
@suzhoum suzhoum marked this pull request as ready for review November 28, 2022 21:15
Copy link
Contributor

@cheungdaven cheungdaven left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@bryanyzhu bryanyzhu left a comment

Choose a reason for hiding this comment

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

LGTM with a minor suggestion for unit test.

@github-actions
Copy link

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

@github-actions
Copy link

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

Copy link
Collaborator

@tonyhoo tonyhoo 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 change. Some follow up actions in general:

  • We should identify any doc/tutorial to be updated with this feature as it allows a new input type and we should make it visible via doc to the end user
  • Can we build docker images for both GPU and CPU to see the inference impact on Sagemaker endpoint? If already done, ignore this point

@@ -156,6 +162,7 @@ def is_numerical_column(
def is_imagepath_column(
data: pd.Series,
col_name: str,
sample_n: Optional[int] = 500,
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's add doc string for this field below as well

multimodal/src/autogluon/multimodal/data/infer_types.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@sxjscience sxjscience left a comment

Choose a reason for hiding this comment

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

LGTM

@suzhoum
Copy link
Contributor Author

suzhoum commented Nov 30, 2022

Thanks for the change. Some follow up actions in general:

  • We should identify any doc/tutorial to be updated with this feature as it allows a new input type and we should make it visible via doc to the end user
  • Can we build docker images for both GPU and CPU to see the inference impact on Sagemaker endpoint? If already done, ignore this point

Thanks for the suggestions! We can mention this feature in our tutorials in a follow up PR. The benchmark was done on a GPU instance and I can build a CPU endpoint as well.

@github-actions
Copy link

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


for img_bytearray in image_bytearrays:
try:
with PIL.Image.open(BytesIO(img_bytearray)) as img:
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the image bytes are encoded by base64?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we enforce that the type passed into the predictor to be the original bytearray? In the cloud serve scripts, I see that currently we would: 1. decode the encoded bytearray in a predefined way, and 2. save to disk to get the path to pass into the predictor. With the new feature, we can save step 2 by assuming no encoding on bytearray. If we are to expect encoded bytearrays in the dataframe, we would probably need to accept a callback decoding function provided by the user. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

base64 is used in encoding images in some cases. I think we'd better support users to provide base64 bytes in dataframe.

Copy link
Collaborator

Choose a reason for hiding this comment

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

base64 is fundamentally a different format than bytearray, since it is a text encoding rather than a binary encoding. We could consider adding a IMAGE_BASE64 column type, or we could try to make a more generic IMAGE_BINARY type that also infers the encoding. But I think this is another feature request separate from the current issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since base64 requires additional decoding, we may define another type IMAGE_BASE64. We can leave this to another PR.

return [_read_byte(os.path.abspath(os.path.join(base_folder, path))) for path in path_l]


def shopee_dataset(
Copy link
Contributor

Choose a reason for hiding this comment

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

Besides unit tests, where do we use this dataset? It doesn't fit the context of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used in a few tutorials at the moment. Do you think creating a datasets.py under the ./utils will help?

Copy link
Contributor

Choose a reason for hiding this comment

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

We usually put the data preparation logic inside the tutorials, e.g., https://auto.gluon.ai/stable/tutorials/multimodal/multimodal_prediction/beginner_multimodal.html. Users can better understand what the data preparation in this way.

Copy link
Contributor

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

github-actions bot commented Dec 1, 2022

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

@suzhoum suzhoum merged commit c1fd4db into autogluon:master Dec 1, 2022
@suzhoum suzhoum added this to the 0.7 Release milestone Feb 16, 2023
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.

None yet

8 participants