-
Notifications
You must be signed in to change notification settings - Fork 110
Description
After installing the 1.6.1 version of the package that only available through the wallgarden, I'd like to thank you for your work, since it actually make us closer to be able to use the linux environment with your technologies.
Here is the feedback and issues I've found to actually get a working system:
Dependencies
The fact that you're limiting to outdated linux distribution is a pain. I can understand you're not delivering open source libraries, but please follow the usual guideline for delivering binaries. Typically, your libraries are linked with 2 problematic dependencies:
- boost filesystem
- stdc++
For the former, you should link statically with it, or simply switch to C++'s standard std::filesystem code. One could solve the issue with downloading the libboost-filesystem-1.74.so file from debian repository and installing it in the lib/python/site-packages/.../voe/libs folder, but it's only a matter of time until debian bumps the version and your code will again fail to link dynamically.
For the latter, it's not a good idea to distribute it, since the other libraries on the system will likely link with the system's libstdc++, thus fail to link when your library make use of the version you're distributing (only a single libstdc++ is allowed in a program's shared library resolution). GCC developpers make great care to ensure libstdc++ is backward compatible, so the system's version will work with your libraries. If it doesn't, providing a libstdc++ that's built for a newer system will cause trouble anyway, since it might use syscall and so on that aren't present on the system. So in short, don't deliver a libstdc++ in your package. I had to delete your lib for shared linking to work.
Mess in virtual environment
You've hand edited the bin/activate script, but forget to apply the same change to the other shell's scripts. I can provide a patch for the shell script I'm using if required.
This is very fragile anyway, since in order to activate/setup the shell is typically hard to get right. One has to first source the /opt/xilinx/xrt/setup before this one, because both modify the LD_LIBRARY_PATH variable in order to find their library and this one doesn't do it right.
The activate script also doesn't add the dependency/lib folder to the LD_LIBRARY_PATH so the model_benchmark binary has to be tweaked to run.
In the end, the use of the environment variable for storing path is IMHO, clumsy. I don't see how the lemonade server could use the package with all this messing environment variables. Maybe you should use a config file instead that's specifying where the other files are, something like "/etc/amd/ryzenai.conf", so a third-party software isn't polluted with a longish LD_LIBRARY_PATH that would break its libraries.
Even better, create a /lib/ryzenai folder to store your libraries and add the path to a ldconfig script (in the usual /etc/ld.so.conf.d/ folder). So it doesn't require a messy hack to run them.
Missing actual software in the package
One of the main use of this package is to be able to use the NPU on linux. Yet, there are no software making this possible in the package. The only software (quicktest, model_benchmark) are both working (great!) but are useless from the user point of view. The LLM's run_model.py doesn't accept a OGA model (which is the point of the package/NPU, right?) even after hand editing the genai_config.json to replace the Windows's dll by a linux .so file (while model_benchmark works fine with the same model!).
So after all the trouble of installing and fixing the mess, we have nothing to actually play with, except a benchmark that just tease us but with no outside world use.
OGA's genai_config.json is windows only
There are too many windowsity in the downloaded model. Lemonade-server can pull the model just fine, but it can't run it (it's not even listed). It has to be hand edited to remove the ".dll" in the file and replace them with the equivalent ".so" file. There's also a "directx_xrt" lurking around I don't know if I need to change it or not. Maybe the base of the library should be used instead (so "onnx_custom_ops" instead of "onnx_custom_ops.dll") and let the actual software which knows what platform it is running on, sort out the actual filename, it's not hard to append a prefix and a suffix to a string.
Also the delivered python code (like run_model.py) doesn't understand the genai_config structure and chokes on unexpected keys in the json object. This means it wasn't even tested before delivery with an actual model 😢
No stable diffusion example
In the previous version and in the documentation, there's a stable diffusion example to use GenAI. But the delivered package doesn't contain it anymore. Can I use the SD models ? If so, how?
Ignored by lemonade_server
I'm not sure I've done everything right with this one, but it seems lemonade_server is completely ignoring the package. If I made a mistake in configuring it, please tell me how to fix it?