Skip to content

Make ModelOutput serializable#26493

Merged
ydshieh merged 2 commits intohuggingface:mainfrom
cbensimon:patch-1
Oct 5, 2023
Merged

Make ModelOutput serializable#26493
ydshieh merged 2 commits intohuggingface:mainfrom
cbensimon:patch-1

Conversation

@cbensimon
Copy link
Contributor

@cbensimon cbensimon commented Sep 29, 2023 •
edited
Loading

Currently, @dataclass ModelOutput instances can't be pickled, which can be inconvenient in some situations
This PR fixes this by adding a custom __reduce__ method to the ModelOutput class

Original PR from diffusers : huggingface/diffusers#5234

EDIT: Actual use case for me is passing a ModelOutput instance in a multiprocessing queue
(this is needed if a model __call__ is wrapped inside the ZeroGPU decorator : model = spaces.GPU(model))

@cbensimon cbensimon marked this pull request as draft September 29, 2023 14:42
@cbensimon cbensimon mentioned this pull request Sep 29, 2023
Merged
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 29, 2023 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

@cbensimon cbensimon marked this pull request as ready for review October 3, 2023 09:36
ArthurZucker
ArthurZucker approved these changes Oct 3, 2023
View reviewed changes
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As per the review in diffusers, good addition IMO.
WDYT @LysandreJik.

Could you add in the PR descripition the issue you were facing before (to have a real use case 😉 )

@ydshieh
Copy link
Collaborator

ydshieh commented Oct 3, 2023 •
edited
Loading

passing a ModelOutput instance in a multiprocessing queue

I have experience regarding things become slow (or even blocked) when passing (large) tensors between processes.

But maybe things change along time and this is no longer a common issue.

@cbensimon
Copy link
Contributor Author

cbensimon commented Oct 3, 2023

passing a ModelOutput instance in a multiprocessing queue

I have experience regarding things become slow (or even blocked) when passing (large) tensors between processes.

But maybe things change along time and this is no longer a common issue.

Sure, the PR is more here by correctness / symmetry because the same thing is done on diffusers side (but I agree it makes more sense in diffusers since it's a pipeline output while it's a model output in transformers)

LysandreJik
LysandreJik approved these changes Oct 4, 2023
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM!

@ydshieh ydshieh merged commit 19f0b7d into huggingface:main Oct 5, 2023
@cbensimon cbensimon deleted the patch-1 branch October 16, 2023 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ydshieh ydshieh ydshieh approved these changes

@LysandreJik LysandreJik LysandreJik approved these changes

@ArthurZucker ArthurZucker ArthurZucker approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cbensimon @HuggingFaceDocBuilderDev @ydshieh @LysandreJik @ArthurZucker