WasmEdge icon indicating copy to clipboard operation
WasmEdge copied to clipboard

[Wasi-nn] PyTorch AOTI

Open LFsWang opened this issue 1 year ago • 4 comments

https://pytorch.org/docs/stable/torch.compiler_aot_inductor.html

Add a support to load the AOTInductor for PyTorch.

Note: use AOTInductor build by pip need the wasmedge with _GLIBCXX_USE_CXX11_ABI=0

  • [x] Pump PyTorch from 1.8.0 to 2.4.1
  • [x] Check old examples work
  • [x] Merge this with origin PyTorch plugin

LFsWang avatar Sep 03 '24 07:09 LFsWang

Hello, I am a code review agent on flows.network. Here are my reviews of changed source code files in this PR.


plugins/wasi_nn/wasinn_torch.cpp

Potential issues

N/A

Summary of changes

N/A

plugins/wasi_nn/wasinn_torch.h

Potential issues

N/A

Summary of changes

N/A

plugins/wasi_nn/wasinnenv.cpp

Potential issues

  1. Issue 1: The BackendMap contains duplicate entries for "pytorch" and "pytorchaoti" both mapping to Backend::PyTorch. This redundancy is unnecessary and can lead to confusion.

  2. Issue 2: The load function does not handle partial reads, where the number of bytes read from the file does not match FileSize. This could result in incomplete data being loaded if File.read() fails to read all expected bytes.

  3. Issue 3: The ModelPathData is constructed using a string prefixed with "preload:". If the paths are long or numerous, this could lead to significant memory usage and potential performance issues when storing and processing these models.

Summary of changes

    • Added "pytorchaoti" as a new key in BackendMap, mapping it to the existing Backend::PyTorch value.
  • Modified the condition in WasiNNEnvironment constructor to include an additional check for Backend::PyTorch when Encode is "pytorchaoti", ensuring specific handling similar to GGML.

juntao avatar Sep 03 '24 07:09 juntao

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.70%. Comparing base (31920fa) to head (8fc0664). Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3724   +/-   ##
=======================================
  Coverage   78.70%   78.70%           
=======================================
  Files         272      272           
  Lines       36777    36777           
  Branches     6545     6539    -6     
=======================================
  Hits        28944    28944           
- Misses       6346     6350    +4     
+ Partials     1487     1483    -4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 04 '24 05:09 codecov[bot]

note: for manylinux_2_28_x86_64 , use add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0) and install pytorch with no c++11 abi will work

LFsWang avatar Oct 08 '24 09:10 LFsWang

note: for manylinux_2_28_x86_64 , use add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0) and install pytorch with no c++11 abi will work

I see, so there is still a compatible issue about the C++11 ABI in manylinux_2_28.

hydai avatar Oct 08 '24 10:10 hydai