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

--accelerator--device #399

Merged
merged 4 commits into from
Jan 29, 2023

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Jan 28, 2023

内容

 Usage: download [OPTIONS]

 Options:
       --min
           ダウンロードするライブラリを最小限にするように指定
   -o, --output <OUTPUT>
           出力先の指定 [default: ./voicevox_core]
   -v, --version <VERSION>
           ダウンロードするvoicevox_coreのバージョンの指定 [default: latest]
       --additional-libraries-version <ADDITIONAL_LIBRARIES_VERSION>
           追加でダウンロードするライブラリのバージョン [default: latest]
-      --accelerator <ACCELERATOR>
-          ダウンロードするacceleratorを指定する(cudaはlinuxのみ) [default: cpu] [possible values: cpu, cuda, directml]
+      --device <DEVICE>
+          ダウンロードするデバイスを指定する(cudaはlinuxのみ) [default: cpu] [possible values: cpu, cuda, directml]
       --cpu-arch <CPU_ARCH>
           ダウンロードするcpuのアーキテクチャを指定する [default: x64] [possible values: x86, x64, arm64]
       --os <OS>
           ダウンロードする対象のOSを指定する [default: linux] [possible values: windows, linux, osx]
   -h, --help
           Print help information
  INFO 対象OS: linux
  INFO 対象CPUアーキテクチャ: x64
- INFO ダウンロードアーティファクトタイプ: cuda
+ INFO ダウンロードデバイスタイプ: cuda
  INFO ダウンロードvoicevox_coreバージョン: 0.14.0-preview.4
  INFO ダウンロード追加ライブラリバージョン: 0.1.0

関連 Issue

その他

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

GPU CPUがアクセラレーター、CPU CUDA DirectMLがデバイス、という感じかなと!

Co-authored-by: Gray Suitcase <[email protected]>
@PickledChair
Copy link
Member

PR で提出された範囲は問題なさそうでした!

ちょっと他の部分も調べた限りでは、次の2点が気になりました:

まだしばらくダウンロードスクリプトが運用されるのであれば変更の必要がありそうですが、 #375 (comment) を参照する限り、近い将来にスクリプトがなくなりそうでもあり、Rust 版ダウンローダーに切り替えるタイミングによるのかな、とも感じました。とはいえ、変更した方が無難そうです。

@qryxip
Copy link
Member Author

qryxip commented Jan 29, 2023

#375 から続く流れなので、別PRを作ってみます。

@qryxip
Copy link
Member Author

qryxip commented Jan 29, 2023

あ、いや違うdeviceにするのはこのPRか

@qryxip
Copy link
Member Author

qryxip commented Jan 29, 2023

更新しました。ただしPowerShell版が動作するかどうかは確かめてません。

Copy link
Member

@PickledChair PickledChair left a comment

Choose a reason for hiding this comment

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

LGTM!

Rust版ダウンローダーとdownload.shについては試してみましたが大丈夫そうでした。また、ほぼキーワードの置換のみ行われており、device が特別なキーワード等でない限りは問題ないと思われるので、マージしてしまいます……!

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.

3 participants