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 an initial warmup step to IPEXModels #543

Merged
merged 7 commits into from
Jan 31, 2024
Merged

Add an initial warmup step to IPEXModels #543

merged 7 commits into from
Jan 31, 2024

Conversation

ofirzaf
Copy link
Contributor

@ofirzaf ofirzaf commented Jan 31, 2024

What does this PR do?

The first 2 forwards of an IPEXModel after trace/load includes background optimizations steps that make the output of these forwards unpredictable and non consistent with the model after the optimizations. To fix that, an initial warmup step was added to the __init__ of IPEXModels

Depends on PR #542

@echarlaix

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Collaborator

@echarlaix echarlaix left a comment

Choose a reason for hiding this comment

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

LGTM

optimum/intel/ipex/modeling_base.py Outdated Show resolved Hide resolved
optimum/intel/ipex/modeling_base.py Outdated Show resolved Hide resolved
Comment on lines 295 to 297
@wraps(IPEXModel.forward)
def forward(self, *args, **kwargs):
outputs = self.model(*args, **kwargs)
outputs = super().forward(*args, **kwargs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prepare_jit_inputs looks at the signature of the function and the wraps and super help avoid code copy

Copy link
Collaborator

Choose a reason for hiding this comment

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

would prefer we avoid as it will fail in case outputs is not a dict

return ModelOutput(**outputs) if isinstance(outputs, dict) else ModelOutput(logits=outputs[0])

Copy link
Collaborator

Choose a reason for hiding this comment

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

also not sure to see the link with prepare_jit_inputs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. In _init_warmup we call prepare_jit_inputs which examines the passed model's forward signature to see which dummy inputs exists in the signature. If we don't use wraps we get the signature of
    self, *args, **kwargs
    
    instead of
    self, input_ids: torch.Tensor, attention_mask: torch.Tensor, token_type_ids: torch.Tensor = None, **kwargs,
    
  2. outputs will always be a dict because this is the output of IPEXModel.forward, no?

Copy link
Collaborator

@echarlaix echarlaix Jan 31, 2024

Choose a reason for hiding this comment

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

  1. OK I understand, was thinking that prepare_jit_inputs was only used for the torchscript export but I see that it's also used in _init_warmup, thanks for the clarification
  2. here I'm talking about outputs https://github.com/huggingface/optimum-intel/blob/8ee487dc2ade5bd0023d1bbe0a0103d6af8821e0/optimum/intel/ipex/modeling_base.py#L192C9-L192C16

@echarlaix echarlaix merged commit 788e458 into huggingface:main Jan 31, 2024
7 of 10 checks passed
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.

3 participants