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

directml用release buildを作成 #210

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

qwerty2501
Copy link
Contributor

内容

directmlをサポートするためのPRです

関連 Issue

refs #128 VOICEVOX/onnxruntime-rs#1

その他

VOICEVOX/onnxruntime-rs#1 が mergeされてからreviewすること
以下の確認が必要

  • ローカルでdirectml向けのbuildが行えること
  • CIからdirectmlのrelease buildが行われ、 releasesにdirectmlが含まれること

@Hiroshiba
Copy link
Member

onnxruntime-rsの方、マージしました!
ついでにgithub actionsのテストがfailedだったので回し直しました。

@qwerty2501
Copy link
Contributor Author

あ、 cargo test するさいに --all-featuresがしている影響でlinuxやmacのtestが失敗するようですね・・・

@qwerty2501
Copy link
Contributor Author

qwerty2501 commented Aug 1, 2022

現状cargoでは特定OSのみに特定のfeatureを有効にする方法がないため、onnxruntimeのほうでwindowsの場合のみにdirectml版のダウンロードを行うようにするupdate PRをつくりました
VOICEVOX/onnxruntime-rs#2

merge後をみこしてバージョンアップしていますので再度テストを実行する必要があります

@qwerty2501
Copy link
Contributor Author

@Hiroshiba @PickledChair テスト通るようになりました

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

.github/workflows/test.yml Show resolved Hide resolved
@@ -58,7 +68,7 @@ jobs:
cargo build
find target/debug/build/onnxruntime-sys-*/out/onnxruntime_*/onnxruntime-*/lib -name onnxruntime.dll -ctime 0
find target/debug/build/onnxruntime-sys-*/out/onnxruntime_*/onnxruntime-*/lib -name onnxruntime.dll -ctime 0 | head -n 1 | xargs -i cp {} target/debug/deps/
- run: cargo test --all-features
- run: cargo test --features ${{ matrix.features }}
Copy link
Member

Choose a reason for hiding this comment

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

新しくfeatureができたとき、ここのテストに足し忘れる(設定し忘れる)のって防止できたりしそうでしょうか。
(たぶん防止方法は無いと思っていて、気をつけようかなと思っています)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

これは気をつけるしか無いですね

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!

1つ質問なのですが、C++ 版の時と違って配布物として onnxruntime.dll を同梱していると思うのですが、DirectML.dll は同梱しない感じでしょうか?(必要に応じて DirectML.dll が delay load される仕組みのようなので、現状は CPU 実行であれば onnxruntime.dll 単独でも実行できる感じになっているようです。)

@qwerty2501
Copy link
Contributor Author

まだ必要なdllがあるんですか?
どこからダウンロードするかもわかってないですが、実装するとしたらまた結構な時間かかりそうです。

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 2, 2022

「core.zipをダウンロードすれば即使える」か、「そのあといろいろ揃えれば使える」か、どっちを目指すかによりそうです。

今は、CPU版は前者で、CUDA版やDirectML版は後者という状態(CPUでは動く)。
で、CUDAはとても重たい・・・。

まあ、DirectML.dllやcuda.dllは同梱しなくて良いかなと思いました!

そういう意味ではonnxruntime.dllもなくて良いかもですが、あってもいいので、せっかくだから付けちゃいましょう・・・!
いやonnxruntime.dllだけが必須なんでした。今の状態が一番いいと思います!!

@Hiroshiba
Copy link
Member

ということでマージします!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

@Hiroshiba Hiroshiba merged commit 6dfaa72 into VOICEVOX:rust Aug 2, 2022
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