VOICEVOX / voicevox_core

無料で使える中品質なテキスト読み上げソフトウェア、VOICEVOXのコア
https://voicevox.hiroshiba.jp/
MIT License
857 stars 115 forks source link

ONNX Runtimeとモデルのシグネチャを隔離する #675

Closed qryxip closed 10 months ago

qryxip commented 10 months ago

内容

ONNX Runtimeとモデルのシグネチャ(?)の存在を、別のモジュールに分離します。

以下の改革の前準備です。

https://github.com/VOICEVOX/voicevox_core/pull/553#issuecomment-1656366892

関連 Issue

545

その他

Hiroshiba commented 10 months ago

なかなか巨大になりそうですね…!! お待ちしてます!!

qryxip commented 10 months ago

大体考えて組んだので、draftを外します。

モジュールinferの登場人物は主に3つに分けられます。これらをもとに抽象化を組んでいます。 (ただinfer.rsは115行しかないといっても、中身の圧が強いのでファイル分割すべきかもしれません)

補足としては:

qryxip commented 10 months ago

色々再構成しました。 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/e0f29c649df39a4006a47c8813208ac9232cb9d1 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/cb1db348ab43b01b542c6404e6c0283e9aae251b https://github.com/VOICEVOX/voicevox_core/pull/675/commits/c3e08dd4da27b9691203f071c2bf716badf1e087 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/e4b91abbf4fa95711a9f62f72e5fcf00b0735999 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/8584d2727252c1bd6971f01150386ebd3adc73e7 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/47953098a19c70f252e210dcff3e8b9e1843f1a1 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/a5dbbddaa3109d7c859f30feadc2fd89900c41d2 https://github.com/VOICEVOX/voicevox_core/pull/675/commits/525f4b1821fff4dc67b0469497d3e4213f5e150a

主な変更点として:

qryxip commented 10 months ago

https://github.com/VOICEVOX/voicevox_core/pull/675/commits/8b4f3b6f4f5be8a083eb22427c031ba53ae85cfd Signatureの"kind"は、"model kind"ということにしました。というのもこれをキーにしたmapでsessionを管理したりするので、"model"と呼ぶ方が実態と合っているかなと思ったので。 (それに伴いtrait InferenceModelGroupという概念を追加しました)

qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago

であれば、もう一旦マージして、階層構造を小さくすることに取り組むのが良いのかなと思いました。 実装していく上でまた新しい課題が見つかったりもする可能性も全然あると思いますし。

構造の変更はもうこのプルリクエスト内ではやめて、名称の変更やドキュメントの追加だけにして、次に進むというのはどうでしょう・・・? (1回のレビューに3時間かかっており。。。。)

はい。それがよいと思います。

階層構造ですが、今InferenceCoreでやっている「入出力のworkaround処理」をdomain.rs(旧signatures.rs)に移せれば、こんな感じにできるのではと思っています。

synthesizer.rs --+--> infer/status.rs -----> infer/runtimes/
                 +--> infer/domain.rs
                 |
                 +--> open_jtalk.rs
Hiroshiba commented 10 months ago

こんな感じにできるのではと思っています。

なるほどです、良さそうに思いました!!

なんとなくの直感ですが、statusがかなり多くの責務を担いそうだなと思いました。statusという名称が曖昧なので便利屋さんみたいになりがちかも。将来なんか名前変えられると良さそう! あとSynthesizerが何を見れるべきかを考えていくとアーキテクチャ作りやすいのかなとか思いました。RuntimeやSessionは見れるべきではない、など?

qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
qryxip commented 10 months ago
Hiroshiba commented 10 months ago

修正確認しました!!マージします!!!