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

BasePhoneme.onehot() の出力型変更 #788

Closed
3 tasks done
tarepan opened this issue Nov 27, 2023 · 3 comments · Fixed by #810
Closed
3 tasks done

BasePhoneme.onehot() の出力型変更 #788

tarepan opened this issue Nov 27, 2023 · 3 comments · Fixed by #810

Comments

@tarepan
Copy link
Contributor

tarepan commented Nov 27, 2023

内容

要望: BasePhoneme.onehot() の出力型変更

voicevox_engine ではモデル用音素表現としてonehotベクトルを利用しており、BasePhoneme はそのためのメソッド .onehot() を有している。
しかし現在の実装では .onehot() は一切利用されず、必要箇所で個別に実装している。
また現在のBasePhoneme.onehot()はBool型のベクトルを出力するため、Fp32ベクトルが必要な上記の個別実装をそのままは置き換えられない。

よって BasePhoneme.onehot() の出力型変更を提案します。

Pros 良くなる点

  • 重複コード除去による見通し改善

Cons 悪くなる点

  • (未使用内部関数の)後方互換性破壊

実現方法

  • True/False から 0./1. への置き換え
  • 個別実装の置き換え

VOICEVOXのバージョン

0.14.10

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux
@github-actions github-actions bot added OS 依存:linux Linux に依存した現象 OS 依存:mac macOS に依存した現象 OS 依存:win Windows に依存した現象 labels Nov 27, 2023
@Hiroshiba
Copy link
Member

提案ありがとうございます!!

BasePhoneme.onehot() の出力型変更を提案します。

よくよく考えると、音素クラスにonehot化のメソッドが生えているのは若干役割違いかもと少し思いました!
synthesis_engine内辺りにonehotを出力する関数を作ってもいいかも?

まあどちらにせよ切り出すのは賛成で、今のmasterブランチの実装よりは提案の形のが良いかなと感じました!!

@tarepan
Copy link
Contributor Author

tarepan commented Nov 28, 2023

👍
#790 に依存しているため、当該PRマージ後に着手します。

@tarepan
Copy link
Contributor Author

tarepan commented Dec 5, 2023

着手しました。

@tarepan tarepan removed OS 依存:mac macOS に依存した現象 OS 依存:linux Linux に依存した現象 OS 依存:win Windows に依存した現象 labels Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants