Support for decoder pipeline based multimodel#1775
Support for decoder pipeline based multimodel#1775prudhvi-qti wants to merge 6 commits intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree company="Qualcomm" |
| state_.outputs_[index_] = other.state_.inputs_[other.index_]; | ||
| } | ||
|
|
||
| WindowedEmbeddings::WindowedEmbeddings(State& state, Embeddings::Mode mode, const std::string& name) |
There was a problem hiding this comment.
I think this can be reduced to just the following because it's so similar to the parent class constructor:
WindowedEmbeddings::WindowedEmbeddings(State& state, Embeddings::Mode mode, const std::string& name)
: Embeddings(state, mode, name) {
window_size_ = model_.config_->model.decoder.sliding_window->window_size;
}| const uint16_t* full_data = full_embeddings->GetTensorData<uint16_t>(); | ||
|
|
||
| if (window_index_ == 0) { | ||
| num_windows_ = (sequence_length + window_size_ - 1) / window_size_; |
There was a problem hiding this comment.
the first two conditions here share a decent amount of initialization? could we combine these two conditions and remove some duplicate code?
| void Update(Embeddings& embeddings); | ||
|
|
||
| private: | ||
| State& state_; |
There was a problem hiding this comment.
we could in theory update the parent class variables to be protected and then these don't need to be redeclared in the child class
| config_session_options.provider_options, is_primary_session_options, | ||
| disable_graph_capture, *config_); | ||
|
|
||
|
|
||
| namespace Generators { | ||
|
|
||
| struct MultiModalPipelineLanguageModel : Model { |
There was a problem hiding this comment.
would suggest trying to align the name of the class here and the name of the file
|
|
||
| namespace { | ||
|
|
||
| int64_t GetNumImageTokens(const std::vector<ExtraInput>& extra_inputs) { |
There was a problem hiding this comment.
only usage of this function in file is commented out
| return 0; | ||
| } | ||
|
|
||
| int64_t GetNumAudioTokens(const std::vector<ExtraInput>& extra_inputs, |
There was a problem hiding this comment.
would be nice if we could move these to another file where they can both be accessed by this file and the multi_modal.cpp file
|
|
||
| } // namespace | ||
|
|
||
| MultiModalPipelineLanguageModel::MultiModalPipelineLanguageModel(std::unique_ptr<Config> config, OrtEnv& ort_env, bool vision, bool speech) |
There was a problem hiding this comment.
feels like with the rest of this file there is a big opportunity for code reuse with multi_modal.cpp if we organize the classes a little better
prudhvi-qti
force-pushed
the
prudhvi-qti/multimodel-decoding-pipeline
branch
from
bed9e8a to
78dc399
Compare
prudhvi-qti
force-pushed
the
prudhvi-qti/multimodel-decoding-pipeline
branch
from
d425916 to
588b0d1
Compare
2 participants
No description provided.