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

Upgrade RedisAI to 1.2.7 #234

Merged
merged 8 commits into from
Oct 24, 2022
Merged

Upgrade RedisAI to 1.2.7 #234

merged 8 commits into from
Oct 24, 2022

Conversation

ashao
Copy link
Member

@ashao ashao commented Sep 21, 2022

With the release of RedisAI 1.2.7, we can bump the supported versions of pyTorch, Tensorflow, and ONNX to more recent versions. Additionally, while updating the dependencies for RedisAI 1.2.7, the installation of the pyTorch backends with GPU support should now be more robust because we pin a more prescriptive version of torch and torchvision.

The new versions were tested on an internal HPE machine with on V100s.

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2022

Codecov Report

Merging #234 (eda326e) into develop (be6dfc3) will decrease coverage by 0.06%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #234      +/-   ##
===========================================
- Coverage    84.31%   84.25%   -0.07%     
===========================================
  Files           56       56              
  Lines         3284     3284              
===========================================
- Hits          2769     2767       -2     
- Misses         515      517       +2     
Impacted Files Coverage Δ
smartsim/settings/containers.py 80.95% <0.00%> (-6.35%) ⬇️
smartsim/_core/launcher/step/localStep.py 86.66% <0.00%> (-3.34%) ⬇️
smartsim/settings/base.py 93.75% <0.00%> (-0.70%) ⬇️
smartsim/_core/launcher/taskManager.py 82.24% <0.00%> (+2.36%) ⬆️

* - ONNX_
- 1.9
* - ONNX_ Runtime
- 1.11.1
Copy link
Contributor

Choose a reason for hiding this comment

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

If these updates require RAI v1.2.7, we shouldn't claim that we still support older versions. There is some ambiguity here as to whether we're pushing everything forward or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

run_tests.yml still tries to test with older versions of RedisAI

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, we are keeping 1.2.3, 1.2.5, 1.2.7. I've updated the table to reflect the various versions we support

@ashao ashao marked this pull request as ready for review October 19, 2022 18:32
Copy link
Contributor

@billschereriii billschereriii left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this. Some feedback that I think needs to be addressed first. It's also unclear to me whether we're trying to support older versions of RedisAI or not -- some of the code does support older versions, but some seems to only support v1.2.7

Copy link
Contributor

@billschereriii billschereriii left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for taking care of this and cleaning up the code to get more production-ready

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.

Looks fantastic seems to build for me! LGTM!!

@ashao ashao merged commit 4c61fae into CrayLabs:develop Oct 24, 2022
@ashao ashao deleted the redisai_upgrade branch November 9, 2022 00:21
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.

4 participants