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

TYP: numpyの型情報を追加 #988

Merged
merged 9 commits into from
Jan 7, 2024

Conversation

sabonerune
Copy link
Contributor

内容

numpyのdtypeの型情報を追加してみました。

その他

かなり緩く付けたためあまり恩恵がないかもしれない。

import numpyの多くをimport numpy as npに置き換えた。

Copy link

github-actions bot commented Jan 7, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 514 328 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 225 159 coverage-29%
voicevox_engine/dev/core/init.py 2 0 coverage-100%
voicevox_engine/dev/core/mock.py 37 8 coverage-78%
voicevox_engine/dev/tts_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/tts_engine/mock.py 29 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 28 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 199 11 coverage-94%
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 2301 729 coverage-68%

Copy link
Contributor Author

@sabonerune sabonerune left a comment

Choose a reason for hiding this comment

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

ほぼ型情報の追加ですが挙動を変えた部分にコメントしました。

Comment on lines -41 to +42
return wave.astype("int16")
return wave
Copy link
Contributor Author

Choose a reason for hiding this comment

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

エンジンの型と合わないため動作を変えました。
エンジンではnpt.NDArray[np.float32]npt.NDArray[np.float64]を返すはず。

Copy link
Member

Choose a reason for hiding this comment

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

ちょっと自信ないのですが、値域が違うかもです!
float32のwavは-1から1、int16は…ちょっとわかりませんが、そのあたり変換しないとすごい音量になるかもです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。
soundfileが自動的に識別して処理していたことを忘れていました。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

変更した理由
戻り値のdtypeがnp.integernp.int16だとmypyが通らない。

Copy link
Member

Choose a reason for hiding this comment

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

なるほどです!
たぶんfloatにした後2の15乗くらいで割ればいい感じになる気がします!(自信若干なし)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

今思いついたのですがtts_engineの型情報の方を緩めればいいことに気づきました。

Copy link
Member

Choose a reason for hiding this comment

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

普通はmock側を実体に合わせるので、mockに合わせて型変えるのはちょっと逆かもです!

voicevox_engine/tts_pipeline/tts_engine.py Show resolved Hide resolved
@sabonerune sabonerune marked this pull request as ready for review January 7, 2024 13:49
@sabonerune sabonerune requested a review from a team as a code owner January 7, 2024 13:49
@sabonerune sabonerune requested review from y-chan and removed request for a team January 7, 2024 13:49
@sabonerune
Copy link
Contributor Author

問題があったのでcloseします

@sabonerune sabonerune closed this Jan 7, 2024
@Hiroshiba
Copy link
Member

Hiroshiba commented Jan 7, 2024

closeされてますが、ぜひ型をつけたいでね!!

もし可能であれば、先に挙動が変わる部分の変更を入れた後に型追加できるとこちら的にありがたいかもです!
(そのままでもまあ大丈夫です!)

@sabonerune
Copy link
Contributor Author

修正したので再度OPENします

@sabonerune sabonerune reopened this Jan 7, 2024
@Hiroshiba Hiroshiba requested review from Hiroshiba and removed request for y-chan January 7, 2024 16:45
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.

すみません!!!
npt.NDArray -> NDArrayに、np.integerをnp.int64にする作業が可能なのか試していたのですが、間違えてpushしてしまいました 🙇 🙇 🙇

@@ -24,15 +24,15 @@
class MorphingParameter:
Copy link
Member

Choose a reason for hiding this comment

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

この上のFIXME消せそうな気がしますね!

return wave * query.volumeScale


_SoxrType = TypeVar("_SoxrType", np.float32, np.float64, np.int16, np.int32)
Copy link
Member

Choose a reason for hiding this comment

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

ここはfloatingで良くなった・・・?

@@ -203,14 +208,19 @@ def apply_output_sampling_rate(
return wave


def apply_output_stereo(wave: ndarray, query: AudioQuery) -> ndarray:
T = TypeVar("T", bound=np.generic)
Copy link
Member

Choose a reason for hiding this comment

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

ここもfloatingでよくなった感じですかね

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!!!

良いですね!!
ちょっとこっち側で、np.floatingや残ってるndarrayに型付けさせていただこうと思います!

今考えてるなんとなくの方針は、コアはfloat32とint64しか受け付けないのでその入出力はそれに合わせるつもりです。
あとpyorldの出力だけがfloat64になっているのですが、もうこれは別に品質に寄与しないので、全部float32で固定で良いかなと思ってます。

とにかくありがとうございました!!

@Hiroshiba Hiroshiba merged commit 281f5b3 into VOICEVOX:master Jan 7, 2024
3 checks passed
@sabonerune sabonerune deleted the typ/numpy-type branch January 7, 2024 23:51
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