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

Documented Getting Started Guide for Shark Release #458

Closed
amd-chrissosa opened this issue Nov 8, 2024 · 6 comments · Fixed by #528
Closed

Documented Getting Started Guide for Shark Release #458

amd-chrissosa opened this issue Nov 8, 2024 · 6 comments · Fixed by #528
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@amd-chrissosa
Copy link
Contributor

amd-chrissosa commented Nov 8, 2024

This issue is in preparation to kick off QA to complete by 11/15/2024. The three workflows that need to be documented are the following (assumes user has already done basic installation which has already been documented):

  1. sharktank: User wants to use custom weights to run an optimized punet based model. User downloads weights and uses sharktank to export MLIR. Then user compiles that MLIR with iree-compile and runs it with the iree run module. Existing documentation is here for Punet and IREE compile/run here
  2. SDXL with Shortfin: User is interested in leveraging our high performance inference engine to run SDXL. User installs additional dependencies for SDXL, ensures it's installed correctly, starts a server, and runs the client via an example json file. Point to API to show how the json can be changed to use SDXL. Existing documentation is here and API for the server is still in code here
  3. Llama with SGLang: User is interested in leveraging our high performance inference engine to run Llama (but interacts with it through SGLang since the user interface in Shortfin is still fairly small compared to SGLang). SGlang should be able to connect to shortfin. Should be able to just launch SGLang and connect to shortfin (sitting on top) using @stbaione's branch. Instructions to use here: https://gist.github.com/stbaione/2843eced1b8c1042127bec3ca8774d9e
@amd-chrissosa amd-chrissosa self-assigned this Nov 8, 2024
@amd-chrissosa amd-chrissosa added the documentation Improvements or additions to documentation label Nov 8, 2024
@amd-chrissosa amd-chrissosa added this to the 3.0 milestone Nov 8, 2024
@amd-chrissosa amd-chrissosa changed the title Create a User Guide for Shark 3.0 Documented Getting Started Guide for Shark Release Nov 8, 2024
@amd-chrissosa
Copy link
Contributor Author

amd-chrissosa commented Nov 8, 2024

Focus for today

  • Test out SDXL workflow
  • Test out Sharktank workfow with Punet model
  • Address issues found in testing
  • Send out initial PR for User Guide

@amd-chrissosa
Copy link
Contributor Author

amd-chrissosa commented Nov 9, 2024

Wasn't able to get SDXL workflow to work e2e.

What did work:

  • Following the nightly release installation guide I was able to install sharktank as well as shortfin from the last nightly release (from 11/7/2024) in a Python3.11 environment created with pyenv. This included basic tests loading up shortfin and sharktank modules.
  • Installing deps from the shortfin apps page worked.

First issue:

  • python -m shortfin_apps.sd.server --help
Traceback (most recent call last):
  File "<frozen runpy>", line 189, in _run_module_as_main
  File "<frozen runpy>", line 159, in _get_module_details
  File "<frozen importlib._bootstrap_external>", line 1074, in get_code
  File "<frozen importlib._bootstrap_external>", line 1004, in source_to_code
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/sosa/SHARK-platform-v1-workspace/3.11.venv/lib/python3.11/site-packages/shortfin_apps/sd/server.py", line 154
    f"--iree-compile-extra-args={" ".join(ireec_args)}",
                                                       ^
SyntaxError: f-string: expecting '}'

Didn't work. Issue was a bug that @stellaraccident addressed here

Second issue pytest instructions didn't work. In hindsight this makes sense as pytest is path dependent and with a release you are running without a checkout. I've also noticed that both example commands here also rely on paths to scripts in the source checkout that aren't part of the package. I can workaround for now by downloading the source by things like:

python -m shortfin_apps.sd.server --model_config=**./python/shortfin_apps/sd/examples/sdxl_config_i8.json** --device=amdgpu --device_ids=0 --flagfile=**./python/shortfin_apps/sd/examples/sdxl_flags_gfx942.txt** --build_preference=compile

Will not work because of the src paths.

Third issue trying to work around the first issue without a new nightly release. Tried to run pip install . from shortfin instructions. Got error

  /tmp/pip-build-env-svnqb1lj/overlay/lib/python3.11/site-packages/setuptools/dist.py:337: InformationOnly: Normalizing '2.9.0.dev' to '2.9.0.dev0'
        self.metadata.version = self._normalize_version(self.metadata.version)
      -- The C compiler identification is unknown
      -- The CXX compiler identification is unknown
      CMake Error at CMakeLists.txt:21 (project):
        No CMAKE_C_COMPILER could be found.

        Tell CMake where to find the compiler by setting either the environment
        variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
        the compiler, or to the compiler name if it is in the PATH.


      CMake Error at CMakeLists.txt:21 (project):
        No CMAKE_CXX_COMPILER could be found.

        Tell CMake where to find the compiler by setting either the environment
        variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
        to the compiler, or to the compiler name if it is in the PATH.


      -- Configuring incomplete, errors occurred!
      Traceback (most recent call last):
        File "<string>", line 261, in run
        File "<string>", line 277, in build_default_configuration
        File "<string>", line 233, in build_cmake_configuration
        File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
          raise CalledProcessError(retcode, cmd)
      subprocess.CalledProcessError: Command '['cmake', PosixPath('/home/sosa/SHARK-Platform/shortfin'), '-GNinja', '-Wno-dev', '--log-level=VERBOSE', '-DSHORTFIN_BUNDLE_DEPS=ON', '-DCMAKE_BUILD_TYPE=Release', '-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON', '-DPython3_EXECUTABLE=/home/sosa/SHARK-platform-v1-workspace/3.11.venv/bin/python', '-DSHORTFIN_ENABLE_LTO=ON']' returned non-zero exit status 1.
      *** Tracy instrumentation not enabled (enable with SHORTFIN_ENABLE_TRACING=ON)
      setup.py running in cmake build mode:
        SOURCE_DIR = /home/sosa/SHARK-Platform/shortfin
        BINARY_DIR = /home/sosa/SHARK-Platform/shortfin/build
      version_info_rc.json not found. Default to dev build
      Using PACKAGE_VERSION: '2.9.0.dev'
      Found shortfin packages: ['shortfin_apps', '_shortfin', 'shortfin', 'shortfin_apps.llm', 'shortfin_apps.sd', 'shortfin_apps.llm.components', 'shortfin_apps.sd.components', 'shortfin_apps.sd.examples', 'shortfin.array', 'shortfin.interop', 'shortfin.support', 'shortfin.interop.fastapi', 'shortfin.interop.support']
        *********************************
        * Building base shortfin        *
        *********************************
      Removing CMakeCache.txt because Python version changed
      CMake build dir: /home/sosa/SHARK-Platform/shortfin/build/cmake/default
      Configuring with: ['-GNinja', '-Wno-dev', '--log-level=VERBOSE', '-DSHORTFIN_BUNDLE_DEPS=ON', '-DCMAKE_BUILD_TYPE=Release', '-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON', '-DPython3_EXECUTABLE=/home/sosa/SHARK-platform-v1-workspace/3.11.venv/bin/python', '-DSHORTFIN_ENABLE_LTO=ON']
      Native build failed:
      [end of output]

Fourth issue assumed issue was with not having the right setup so followed dev instructions to run dev_me.py. Ran into an issue running CMake

First time configure...
Building
no such file or directory
CMake Error: Generator: execution of make failed. Make command was: /tmp/pip-build-env-mii6rgj2/overlay/bin/ninja
Traceback (most recent call last):
  File "/home/sosa/SHARK-Platform/shortfin/./dev_me.py", line 262, in <module>
    main(sys.argv[1:])
  File "/home/sosa/SHARK-Platform/shortfin/./dev_me.py", line 212, in main
    build_mode(env_info)
  File "/home/sosa/SHARK-Platform/shortfin/./dev_me.py", line 258, in build_mode
    subprocess.check_call([env_info.cmake_exe, "--build", str(build_dir)])
  File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/home/sosa/SHARK-platform-v1-workspace/3.11.venv/bin/cmake', '--build', '/home/sosa/SHARK-Platform/shortfin/build/cmake/default']' returned non-zero exit status 1.

At this point, I think my environment is hosed and need to re-create to get dev setup env working. Will try again over the weekend.

@stellaraccident
Copy link
Contributor

Third issue trying to work around the first issue without a new nightly release. Tried to run pip install . from shortfin instructions. Got error

Looks like you are missing a C compiler. That's not going to work :)

subprocess.check_call([env_info.cmake_exe, "--build", str(build_dir)])

Something wrong with cmake. Needs to log its actual command lines more robustly.

This is a hint: CMake Error: Generator: execution of make failed. Make command was: /tmp/pip-build-env-mii6rgj2/overlay/bin/ninja

What that is saying is that it is trying to use the cmake/ninja from within an isolated venv that pip creates. This is most likely left-over from your prior attempt. Fix would have been to rm -Rf build/. Once it gets wedged like that, it cannot be un-wedged.

@amd-chrissosa
Copy link
Contributor Author

Thanks that's super useful. Deleting the build dir also made it clear I was just missing all the build deps for iree aka sudo apt install cmake ninja-build clang lld got dev_me.py working around issues 3 and 4. Though I'm still seeing errors when loading shortfin.

(3.11.venv) sosa@amd-chrisosa-azure:~/SHARK-Platform/shortfin$ python -m shortfin_apps.sd.server --help
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/sosa/SHARK-Platform/shortfin/python/shortfin_apps/sd/server.py", line 31, in <module>
    from .components.generate import ClientGenerateBatchProcess
  File "/home/sosa/SHARK-Platform/shortfin/python/shortfin_apps/sd/components/generate.py", line 20, in <module>
    from .service import GenerateService
  File "/home/sosa/SHARK-Platform/shortfin/python/shortfin_apps/sd/components/service.py", line 183
    )
    ^
SyntaxError: f-string expression part cannot include a backslash

Taking a look at your previous fix, I'm wondering if 3.11 vs 3.12 just has big string formatting differences so maybe we should just recommend Python 3.12+. Will try with 3.12 to see if it works around the string formatting issues.

@stellaraccident
Copy link
Contributor

stellaraccident commented Nov 9, 2024

F strings are a real work in progress across those python versions. We should just fix them and then run tests. No better way. If you leave non eol versions out, more poor souls will try anyway and have a bad first experience. You don't even need to run tests for this stuff... Just need to load the code.

But we should have a strong recommendation on highest performing setup. Every of the last several versions of python has enabled significant performance improvements. Anyone who cares will want the bleeding edge. But we want oobe and casual use to be ok.

While people can certainly self source on how to get their python env setup, it'd be nice to script the CI better for env setup then recommend people do that for the best outcome. We should just use pyenv for that, which let's us precisely control the python setup. That's what anyone who cares is going to do (either because of perf or because of supply chain management).

@amd-chrissosa
Copy link
Contributor Author

Ack. We should definitely have tests for anything we say we support. I think we need to test anything we say we might support. Right now will test again Mon evening / Tues morning since I'll be OOO for most of Monday

@amd-chrissosa amd-chrissosa linked a pull request Nov 15, 2024 that will close this issue
amd-chrissosa added a commit that referenced this issue Nov 15, 2024
Progress on #458

This PR adds a SHARK user guide to root of docs directory and does some
basic information re-architecture to point installation paths of current
main readmes to the new user guide.

One new landing page and removal of duplicate installation paths in SD
folder to point to both nightly / new release page depending on use
case.

Incorporated a mini guide on supported options for the SD Server /
Client in the user guide.

Changed root readme to include a path for users so that anyone who lands
on the main SHARK readme can quickly get started as a non-developer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants