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

Refactor: _synthesis_impl 前処理の機能別関数化 #815

Closed
3 tasks done
tarepan opened this issue Dec 5, 2023 · 3 comments · Fixed by #907
Closed
3 tasks done

Refactor: _synthesis_impl 前処理の機能別関数化 #815

tarepan opened this issue Dec 5, 2023 · 3 comments · Fixed by #907

Comments

@tarepan
Copy link
Contributor

tarepan commented Dec 5, 2023

内容

要望: _synthesis_impl 内前処理の機能別関数化によるリファクタリング

現在の _synthesis_impl 内における前処理は主に以下の3関数により行われている:

  • calc_frame_per_phoneme()
  • calc_frame_pitch()
  • calc_frame_phoneme()

これらの関数は frame_per_phoneme / pitch / phoneme と扱う対象に基づき適切に分割されている。
一方で関数名の calc_ が暗示するように、その内部に複数の機能を有している。
その機能は以下に大別できる(c.f. #790 review):

  • 補正: Query内グローバル特徴量の適用(例: 音高変更)
  • 拡張: スケール変更(例: モーラスケールからフレームスケールへのexpand)
  • 表現変換: Core向け表現への変換(例: 音素オブジェクトからonehotベクトルへの変換)

これらの機能が1つの calc_() 内に混在している(機能的凝集していない)ため、テスト範囲の増大や重複実装、リファクタリング阻害が起きている。

よって前処理3関数の更なる分割によるリファクタリングを提案します。

Pros 良くなる点

  • 重複実装の削除によるコード減
  • 機能的凝集による見通し改善
  • テスト粒度の小型化によるメンテ性向上

Cons 悪くなる点

  • 無し

実現方法

複数ステップ/PRを介して実装予定。

  • step1: calc_frame_per_phoneme() / calc_frame_pitch() / calc_frame_phoneme() 内の分割
    • change_xxx(): Queryに基づく補正系
    • format_xxx(): Core入力形式への変換系(e.g. float化、onehot化)
    • expand_xxx(): スケール変換系(e.g. モーラ/音素 to フレーム)
  • step2: change_系の calc_ 外移植
  • step3: 重複実装の排除
  • step4: 前処理/後処理の関数化
  • step5: calc_関数の解体

VOICEVOXのバージョン

0.14.10

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

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

提案ありがとうございます!!良い方針に感じました!!

プライベート関数になると思うので、テストをどうやって書くのがいいのかちょっと迷ってました。
doctestでも良いかも?(提案というよりはコメントです!)

名称に関して、こんな感じでもいいかなというのを考えてみました!参考になれば幸いです!

  • change_xxx(): Queryに基づく補正系
    • 引数の内容を適用する、という意味でapplyとかもありかも?
  • format_xxx(): Core入力形式への変換系(e.g. float化、onehot化)
    • 「変換」を素直にconvertでも良いかも?
  • expand_xxx(): スケール変換系(e.g. モーラ/音素 to フレーム)
    • 広がるだけじゃなく縮まる時もありそうかも?rescale・・・?

@tarepan
Copy link
Contributor Author

tarepan commented Dec 5, 2023

プライベート関数になる ... doctestでも良いかも

doctestあまり詳しくないので検討してみます。ありがとうございます。

引数の内容を適用する、という意味でapply ...
「変換」を素直にconvert ...
縮まる時もありそうかも?rescale・・・?

なるほどです、参考にします👍

提案ありがとうございます!!良い方針に感じました!!

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

@tarepan
Copy link
Contributor Author

tarepan commented Dec 6, 2023

step1着手しました。

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