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

必要なライブラリを含めたリリースをできるようにした #246

Closed

Conversation

qwerty2501
Copy link
Contributor

内容

DirectML,cudaでそれぞれ必要なライブラリもリリースに含めるようにした。
cudaについてはサイズが大きいのでzipを分けた。
DirectMLについて、DirectMLなし版であるmin版を追加した

関連 Issue

#213

その他

ドキュメントなどはダウンロードスクリプト実装の際に直そうと思ってます

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.

"CUDAのダウンロードを分ける"というのを、ずっとユーザー側でダウンロードしてもらうんだと思っていました・・・。

今製品版のコアはプライベートリポジトリでビルドしているのですが、artifactとビルド時間に課金が生じます。
(ちなみに無料枠だとartifactはなんと500MBまでしか使えません。)

これは提案なのですが、CUDAとDirectMLは別リポジトリか別releaseにするというのはどうでしょう。
CUDAがartifactを通る過程で圧縮と展開にある程度時間がかかっているのを抑えられるかなと思っています。
あとCUDAのダウンロードと展開は本質的にはコアのビルドと独立しているので、コアの機構がわからなくてもメンテナンスできる人が増え、見通しも良くなるという利点もあります。

プライベートリポジトリはまあ一旦抜きにしても、可能なら分けたほうが良いのかなと思いました。
後出しみたいになってしまってほんと申し訳ないです。。

.github/actions/download-cuda-libraries/action.yml Outdated Show resolved Hide resolved
description: cudaのバージョン
required: true
cuda_lib_dir:
description: libraryは配置されているディレクトリ
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: libraryは配置されているディレクトリ
description: libraryが配置されているディレクトリ

set -eux
CUDA_ROOT=$( echo "${{ steps.cuda-toolkit.outputs.CUDA_PATH }}" | tr '\\' '/' )
mkdir -p download/cuda/bin
cp -v "$CUDA_ROOT/${{ inputs.cuda_lib_dir }}/"*.{so*,dll} download/cuda/bin/ || true
Copy link
Member

Choose a reason for hiding this comment

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

CUDNNはOSごとに処理が別れているので、こちらもそうして引数を少なくすると見通しが良くなりそう?

- os: windows-latest
additional-features: directml
target: x86_64-pc-windows-msvc
artifact_name: windows-x64-directml-min
Copy link
Member

Choose a reason for hiding this comment

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

他と命名規則合わせるために、こっちを無印にして、同梱版を-fullにするのはどうでしょう

Comment on lines +54 to +55
cudnn_download_url: https://developer.download.nvidia.com/compute/redist/cudnn/v8.4.1/local_installers/11.6/cudnn-windows-x86_64-8.4.1.50_cuda11.6-archive.zip
cuda_version: 11.7.0
Copy link
Member

Choose a reason for hiding this comment

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

CUDNNが想定するCUDAのバージョン(11.6?)とCUDAのバージョン(11.7.0)が合ってないかも

@qwerty2501
Copy link
Contributor Author

今製品版のコアはプライベートリポジトリでビルドしているのですが、artifactとビルド時間に課金が生じます。

ああ、なるほどそういうことだったんですね。となるとそもそもこのPRはCloseとしたほうが良いかもですね。

しかし反面CUDAのダウンロードにはものすごく時間がかかるためVoicevoxとして公開リポジトリで管理し、使用するライブラリのみをまとめたresourcesがあると良さそうです。CUDAはインストーラでしか手に入れる手段がない?ようなのでこのままだとダウンロードスクリプトの機能をフルに提供するのは難しくなります。まあもともとのconfigure.pyだとcudaそのもののダウンロードまでやっていなかったようなのですが、DirectMLのダウンロードは行っていたようなので同様にあったほうがよいかなと
DirectMLについてはzipでまとめられそうだったんですが、この際このリポジトリで配布するライブラリは最小限のみとし、追加のライブラリは voicevox_resources といったようなrepositoryのresourceからとってくるという風にしたほうが一貫性はありそうです。また resourceとして提供されていることでダウンロードしやすくなりますから、ダウンロードスクリプトの処理も単純化が狙えてshell scriptとpower shell両方提供するのもそこまで苦ではなくなると思います。

同意していただけるのであればこのままCloseとしていただければと思います。なお voicevox_resources repositoryを作る際にこのPRのCI処理は参考になると思うので branch自体は残しておきます。

@Hiroshiba
Copy link
Member

ありがとうございます!!とても良いと思います!!

voicevox_resources

キャラの画像を配付するリポジトリとして使ってしまっているんですよね・・・。
命名に微妙に困りますね・・・
何が入るかわからないけど何でも入るように、voicevox_other_resourcesとか・・・?

@qwerty2501
Copy link
Contributor Author

まあ名前についてはおまかせしますが、 voicevox_additional_libraries とか?
これがあるとengineのダウンロード処理も簡略化できそうですね

@qwerty2501
Copy link
Contributor Author

閉じても問題なさそうなので閉じます

@qwerty2501 qwerty2501 closed this Aug 17, 2022
@Hiroshiba
Copy link
Member

voicevox_additional_libraries、良いですね!!
将来的にlibrary以外のものも含めたくなりそうですが、そのときは名前変更を検討したいと思います。
voicevox_additional_libraries、とりあえずリポジトリだけ作っちゃいます!

@Hiroshiba
Copy link
Member

https://github.com/VOICEVOX/voicevox_additional_libraries

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