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

Building SmartSim without ML backends #601

Merged
merged 2 commits into from
May 24, 2024
Merged

Building SmartSim without ML backends #601

merged 2 commits into from
May 24, 2024

Conversation

m-kurz
Copy link
Contributor

@m-kurz m-kurz commented May 24, 2024

Currently, SmartSim cannot be built correctly (i.e. with all necessary dependencies) without the ML backends.
To reproduce this, build SmartSim using smart build --no_tf --no_pt and start an Orchestrator with the following Python snippet taken from the tutorials:

from smartsim import Experiment
exp = Experiment("tutorial-smartredis", launcher="local")
db = exp.create_database(db_nodes=1,port=6899,interface="lo")
exp.start(db)                         

The code will fail due to a missing RedisAI installation with the error

smartsim.error.errors.SSConfigError: RedisAI dependency not found. Build with `smart` cli or specify RAI_PATH

The reason is that after RedisAI is built using the smart tool, the resulting library is only installed to the lib folder if and only if the folder backends/ exists, which it does not if no backends are installed. Since after this step the original build folder is deleted and with it the compiled library.

This problem does not occur if any of the backends (TF, PT, ONNX) is installed. However, since they are not needed for many applications it would additional complications and effort to compile them if not necessary. Also compiling RedisAI by itself on pointing RAI_PATH to the installation also works, but poses additional effort.

To circumvent this problem this PR proposes to simply install the RedisAI library by itself if it was built.

…rwise successfully built RedisAI library will be deleted if built as standalone without backends.
@mellis13 mellis13 requested a review from MattToast May 24, 2024 17:03
@mellis13 mellis13 added the area: build Issues related to builds, makefiles, installs, etc label May 24, 2024
Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.61%. Comparing base (54755ad) to head (426f0d4).
Report is 11 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #601      +/-   ##
===========================================
- Coverage    71.53%   67.61%   -3.92%     
===========================================
  Files           78       78              
  Lines         6000     6000              
===========================================
- Hits          4292     4057     -235     
- Misses        1708     1943     +235     

see 46 files with indirect coverage changes

@MattToast MattToast self-assigned this May 24, 2024
Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for catching and fixing this!!

@MattToast MattToast merged commit 7d995bb into CrayLabs:develop May 24, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Issues related to builds, makefiles, installs, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants