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

追加: kana 型 #972

Merged
merged 10 commits into from
Jan 5, 2024
Merged

追加: kana 型 #972

merged 10 commits into from
Jan 5, 2024

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Jan 3, 2024

内容

AquesTalk 風記法音素に型を追加

VOICEVOX 音素の 型付け・validation にあたり、ドメイン相互変換先である AquesTalk 風記法 (kana) ドメインの型付けがまず必要である。
よって型付けをおこなった。

関連 Issue

ref #894

@tarepan tarepan requested a review from a team as a code owner January 3, 2024 12:49
@tarepan tarepan requested review from Hiroshiba and removed request for a team January 3, 2024 12:49
Copy link

github-actions bot commented Jan 3, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 512 327 coverage-36%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/cancellable_engine.py 94 72 coverage-23%
voicevox_engine/core_adapter.py 51 17 coverage-67%
voicevox_engine/core_initializer.py 59 30 coverage-49%
voicevox_engine/core_wrapper.py 224 159 coverage-29%
voicevox_engine/dev/core/init.py 2 0 coverage-100%
voicevox_engine/dev/core/mock.py 36 8 coverage-78%
voicevox_engine/dev/tts_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/tts_engine/mock.py 27 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifest.py 34 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 12 0 coverage-100%
voicevox_engine/engine_manifest/init.py 3 0 coverage-100%
voicevox_engine/library_manager.py 92 5 coverage-95%
voicevox_engine/metas/Metas.py 34 0 coverage-100%
voicevox_engine/metas/MetasStore.py 18 6 coverage-67%
voicevox_engine/metas/init.py 2 0 coverage-100%
voicevox_engine/model.py 162 9 coverage-94%
voicevox_engine/morphing.py 71 46 coverage-35%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 13 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 80 2 coverage-98%
voicevox_engine/preset/init.py 4 0 coverage-100%
voicevox_engine/setting/Setting.py 11 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 17 0 coverage-100%
voicevox_engine/setting/init.py 3 0 coverage-100%
voicevox_engine/tts_pipeline/init.py 3 0 coverage-100%
voicevox_engine/tts_pipeline/acoustic_feature_extractor.py 27 0 coverage-100%
voicevox_engine/tts_pipeline/kana_converter.py 88 1 coverage-99%
voicevox_engine/tts_pipeline/mora_list.py 7 0 coverage-100%
voicevox_engine/tts_pipeline/text_analyzer.py 146 6 coverage-96%
voicevox_engine/tts_pipeline/tts_engine.py 188 8 coverage-96%
voicevox_engine/user_dict.py 145 12 coverage-92%
voicevox_engine/utility/init.py 5 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 13 0 coverage-100%
voicevox_engine/utility/path_utility.py 35 9 coverage-74%
voicevox_engine/utility/run_utility.py 10 7 coverage-30%
TOTAL 2283 725 coverage-68%

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

型導入良いですね!!

言葉がすごい難しいなと感じました。
Graphemeという単語と、Kanaというプレフィックスは、可能なら避けられると語弊を避けられる気がしました!

まだ見きれてないのですが、とりあえず語句周りの提案と認識揃えを 🙇

voicevox_engine/tts_pipeline/mora_list.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/mora_list.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/mora_list.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/mora_list.py Outdated Show resolved Hide resolved
@tarepan
Copy link
Contributor Author

tarepan commented Jan 3, 2024

kana prefix の扱いが全議論の前提になるため、そこの認識合わせがしたいです。

私の認識としては「VOICEVOX ENGINE のコード向け規約として『"AquesTalk 風記法" は コード上で kana に統一』がある」になっています。
以前の名称統一 issue にて、メンテナさんからそのような案内を受けています(#720)。
AquesTalk風記法の変換モジュール名が kana_converter へリネームされたのもこれに沿ったものと認識しています。

これに関する私の認識がズレている、あるいは規約側の方針変更があった、という感じでしょうか…?

@Hiroshiba
Copy link
Member

Hiroshiba commented Jan 3, 2024

@tarepan
あっ、ややこしくてすみません!
認識はあっているのですが、もしprefixにするなら用途が変わるから再考が必要かもという提案でした。

今までのkanaの利用箇所は、AquesTalk風記法をobjectとして扱うときだけだったんですよね。
create_kana(AquesTalk風記法を作成する)とかkana_parser(AquesTalk風記法用のパーサー)とか。

今回のprefixはobjectではなく領域を示すもので、ドメイン用語を使いたい範囲が変わったという認識です。
kana_mora2grapheme(AquesTalk風記法とかの領域で、モーラを書記素にする関数)とか。

今まではkanaがオブジェクトのみでの利用だったのと、AudioQueryなどに含まれる変更不可なプロパティとしてのkanaがあったので、kana続投が都合良かった感じです。
ただ後ろに何がつくかわからないprefixとして使われるのであれば話が別で、誤解されやすくなったため、もしprefixにするのであればより専門的な言い方に変える必要が出てきたように感じたという流れです。

@tarepan
Copy link
Contributor Author

tarepan commented Jan 3, 2024

今までのkanaの利用 ... objectとして扱うときだけ
...
後ろに何がつくかわからないprefix ... 誤解されやすく ... より専門的な言い方に変える必要が出てきた

👍
理解しました。「prefix に流用するには反直感的」ということですね。
新しい prefix 導入無しで済むならそっちがいい、という review comments の方向性も同意です。


大きな論点としては次の2つが残っているかと思います:

  • 用語 Grapheme の是非
  • VOICEVOX Phoneme == Kana's Phoneme

前者について認識の相違が大きそうなので、まずこちらの認識合わせがしたいです。

用語 Grapheme の是非

テキストからモーラに変わってるこの領域では「どう書くか」ではなく「どう読むか」に注目するので、graphemeという言葉は現れない気がします

Grapheme-to-Phoneme (g2p) は典型的なTTSサブモジュールの1つ(参考文献: ref, 参考コード #834)で、「文字に音素を当てる」役割を果たします。VOICEVOX ENGINE であれば AquesTalk 風記法が(第2の)書き文字であり、AquesTalk風記法用の g2p モジュール(パーサー)を介して TTSEngine 向けの音素列へ変換されます。

今回型付けされている mora_list モジュールとそれを用いる kana_converter モジュールではまさに g2p をしています(例↓)

moras.append(_text2mora_with_unvoice[matched_text].copy(deep=True))

よって文字側(AquesTalk 風記法テキストの atom)を Grapheme と呼ぶのは自然と考えます。

その上で、Mora という語彙についてです。
「ヒホ」は「『ヒ』モーラ+『ホ』モーラ」であり、同時に「『h-i』モーラ + 『h-O』モーラ」でもあります。
つまり Grapheme を指すことも、Phonemes を指すこともあります。
なので g2p モジュールで「モーラ」という語を使うと、どちらを指すかわからなくなります。

mora_listkana_converter で「モーラの長さ単位で atoms を扱うよ」という prefix として mora という語を使うのには賛成です。一方で「コ → katakana_mora, ko → mora」という使い分けは(特に音声合成屋に)誤解を招くと感じます。

このような背景から「コ → katakana_mora, ko → mora」ではなく「コ ∈ Grapheme, ko ∈ Phoeneme (Vowel, Consonant)」という方向性の命名をしています。

上記に関して認識がズレていそうな点、上記を受けての命名の方向性についてお聞きできれば嬉しいです。

@Hiroshiba
Copy link
Member

用語 Grapheme の是非
上記に関して認識がズレていそうな点

数点ありそうでした!

kana_converter モジュールではまさに g2p をしています

言葉の定義的にちょっと違和感あるかなと!
まずkanaにはアクセントが含まれていて、phonemeにはアクセントが含まれないので、to phonemeじゃない気がします。やってることはto accent phraseかなと。

もうひとつ、これは素人の感覚ですが、graphemeは自然言語に使う言葉で、記号列であるkanaには使わない気もします。
あと今気づいたのですが、おっしゃる通りgraphemeはTTSの最初のテキスト解析に使われるものなので、知識がある人がこのコードを読んだとき、kanaではなく自然言語の処理だと勘違いするかもです。

このような背景から「コ → katakana_mora, ko → mora」ではなく「コ ∈ Grapheme, ko ∈ Phoeneme (Vowel, Consonant)」という方向性の命名をしています。

moraだとややこしいのは同意です。
僕はko→alphabet_moraにすれば良さそうかなーと思ってます!


自分の考えの根幹は「kanaはgraphemeじゃない気がする」でした!

@tarepan
Copy link
Contributor Author

tarepan commented Jan 3, 2024

やってることはto accent phrase ... kanaはgraphemeじゃない気がする

👍
カタカナ音声文的な表現であって graphemes ではない、ということですね。
妥当な指摘に感じます。

_KanaBaseGrapheme に代わる型としては MoraKatakana が名称として自然に感じました。
KatakanaMoraとどちらが良いでしょうか?(以下例)

MoraKatakana = Literal["ア", "イ", "ウ"]

moras: list[tuple[MoraKatakana, Consonant, Vowel]] = [
    ("ヴョ", "by", "o"),
    ("ヴュ", "by", "u"),
    ...
]

// or 

KatakanaMora = Literal["ア", "イ", "ウ"]

moras: list[tuple[KatakanaMora, Consonant, Vowel]] = [
    ("ヴョ", "by", "o"),
    ("ヴュ", "by", "u"),
    ...
]
> ko→alphabet_mora

👍
「コ → katakana_mora、ko→alphabet_mora」ですね。

@tarepan
Copy link
Contributor Author

tarepan commented Jan 3, 2024

上記議論を受け、「Kana prefix を避け、Grapheme は使わず、Vowel/Consonant を共通化する」方向の修正をおこないました。
その過程で「mora を prefix」&「object として kana」で命名法を統一したところ、スッキリとした印象を受けました。

@Hiroshiba
こちらを Re-review していただき、その中で命名に違和感があれば再度ディスカッションできればと思います。Re-review よろしくお願いします。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

良い感じにまとまったように感じました!!!
いくつか細かい箇所のコメントを書いていますが、概ね良さそうかなと!!

tarepan and others added 2 commits January 4, 2024 22:55
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!!

1点だけ追加でコメントしています!

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
@tarepan
Copy link
Contributor Author

tarepan commented Jan 4, 2024

@Hiroshiba
suggests の commit およびテストパスを確認しました。merge 可能です👍

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@Hiroshiba Hiroshiba merged commit 77a3343 into VOICEVOX:master Jan 5, 2024
3 checks passed
@tarepan tarepan deleted the add/kana_vc_type branch January 5, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants