Support decoder generated with --for-ort from optimum.exporters.onnx in ORTDecoder#554
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
| @@ -372,7 +372,7 @@ def __init__( | |||
| if use_past != self.use_present_in_outputs: | |||
| logger.warning( | |||
There was a problem hiding this comment.
This warning is always raised, why is the warning necessary? cc @michaelbenayoun
There was a problem hiding this comment.
Basically, it is when use_past and use_present_in_outputs do not have the same value. But you are right the current way is not working all the time. Let me fix that on my end on a small PR.
There was a problem hiding this comment.
yes I understand that from the condition! But I'm not sure why there is the warning? In which case would the two be exclusive?
There was a problem hiding this comment.
If you specify use_present_in_outputs=True and use_past=False. Which can be the case when exporting a model which uses cache.
That is true that we should maybe deprecate use_past and have it as a property being:
@property
def use_past(self):
return self.use_past_in_inputs or self.use_present_in_outputs| @@ -372,7 +372,7 @@ def __init__( | |||
| if use_past != self.use_present_in_outputs: | |||
| logger.warning( | |||
There was a problem hiding this comment.
Basically, it is when use_past and use_present_in_outputs do not have the same value. But you are right the current way is not working all the time. Let me fix that on my end on a small PR.
|
@michaelbenayoun I simplified much of the code duplication for multiple ONNX files export, feel free to rereview. |
michaelbenayoun
left a comment
michaelbenayoun
left a comment
There was a problem hiding this comment.
Good refactoring!
I left a few comments, can be merged once they have been considered and/or addressed.
|
All good, thanks. Will leave the tests running and merge once passed. |
4 participants
What does this PR do?
Currently, the
optimum.exporters.onnxexport in separate encoder / decoder files works well. However, for decoder-only models as gpt2, the support was not yet implemented. This PR allows decoder-only models exported throughoptimum.exporters.onnxto be loaded inORTDecodersubclasses (asORTModelForCausalLM).Note: I am not sure it's super clean to duplicate methods like in this PR for specific cases. @echarlaix how do you plan to support the ONNX export for diffusion models in the current framework, and integrate in
optimum.exporters.onnx?There are tests missing to check:
optimum.exporters.onnxis well interfaced with ORTModeloptimum.exporters.onnxworks wellFixes #552