fix(pipelines): support passing decoder model + tokenizer #319
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
We also remove driver reinstallation step
dacorvo
force-pushed
the
fix_pipeline_load_model
branch
from
786e975
to
3d8e2e6
Compare
| @@ -123,7 +123,7 @@ def load_pipeline( | |||
| model, export=export, **compiler_args, **input_shapes, **hub_kwargs, **kwargs | |||
| ) | |||
| # uses neuron model | |||
| elif isinstance(model, NeuronBaseModel): | |||
| elif isinstance(model, (NeuronBaseModel, NeuronModelForCausalLM)): | |||
There was a problem hiding this comment.
QQ: why is NeuronModelForCausalLM not a sublass of NeuronBseModel?
There was a problem hiding this comment.
Because NeuronBaseModel is based on JIT models and implements the corresponding conversion logic. NeuronModelForCausalLM is a subclass of NeuronDecoderModel that uses transformers-neuronx models instead.
We could refactor to add a common class to both though, since the latest subclasses of NeuronBaseModel are overriding pretty much all its methods now.
There was a problem hiding this comment.
Wouldn't it make more sense to use NeuronDecoderModel here?
There was a problem hiding this comment.
I guess it would be good to refactor it if possible. But also it's not pressing and can be postponed to when we have more bandwidth!
There was a problem hiding this comment.
I will wait until @JingyaHuang 's pull-request on T5 is merged as it is a new subclass, and any change I make before that to the base class will result in nightmarish conflicts.
When passing explicitly a neuron model to a pipeline, we check the model class. This modifies the check to accept not only NeuronBaseModel but also NeuronModelForCausalLM.
Update: cherry-picked @glegendre01 modifications to github workflows to reactivate INF1 CI.