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

Support for site-provided dependencies #182

Merged
merged 11 commits into from
Apr 20, 2022

Conversation

ashao
Copy link
Member

@ashao ashao commented Apr 2, 2022

Includes a new function in the smart CLI to aid users whose site admins provide most of the packages. Additionally, if the appropriate environment variables are set, SmartSim will be configured to acknowledge the sitewide package installations.

Sam Partee and others added 3 commits February 12, 2022 14:47
If the SMARTSIM_DEP_PATH environment variable has been set. Base
some of the SmartSim dependencies on that variable instead of the
locally installed libraries.
Create a separate build method for users who have a site-managed
SmartSim installation since most dependencies and libraries will
be handled by the site administrator.
@ashao ashao requested a review from Spartee April 2, 2022 02:16
@ashao ashao force-pushed the smartsim_site_build branch 2 times, most recently from d7756de to 63a6a51 Compare April 2, 2022 12:43
Including a printer for Versioner would require tabulate as an
additional dependency. For now, this functionality has been
removed to avoid errors when installing via pip.
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2022

Codecov Report

Merging #182 (bfde279) into develop (22e0cc4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #182   +/-   ##
========================================
  Coverage    81.73%   81.74%           
========================================
  Files           57       57           
  Lines         2973     2974    +1     
========================================
+ Hits          2430     2431    +1     
  Misses         543      543           
Impacted Files Coverage Δ
smartsim/_core/config/config.py 95.89% <100.00%> (+0.05%) ⬆️

Replace the sitebuild functionality with a `--only_python_packages`
variant of `smart build`. This will also issue a warning for a
case where a user might accidentally omit the device flag (which
defaults to cpu) and not install the GPU variant of torch.
In cases where SmartSim dependencies are provided by the system,
allow Builder objects to detect those locations.
Copy link
Contributor

@Spartee Spartee left a comment

Choose a reason for hiding this comment

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

couple comments. Will do another pass after looking through the site install PR

smartsim/_core/_cli/cli.py Outdated Show resolved Hide resolved
smartsim/_core/_install/builder.py Show resolved Hide resolved
smartsim/_core/config/config.py Outdated Show resolved Hide resolved
@ashao ashao requested a review from Spartee April 15, 2022 03:03
Copy link
Contributor

@Spartee Spartee left a comment

Choose a reason for hiding this comment

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

Approving and should be merged once other PR is merged.

@ashao ashao merged commit 776f926 into CrayLabs:develop Apr 20, 2022
al-rigazzi pushed a commit to al-rigazzi/SmartSim that referenced this pull request May 16, 2022
Support for site-provided dependencies

Include an optional argument`--only_python_packages` to
to `smart build`. This will also issue a warning for a
case where a user might accidentally omit the device flag (which
defaults to cpu) and not install the GPU variant of torch.

The config can now also detect whether dependencies are available
elsewhere on the platform. If so, those libraries and executables are
not built installed within the user's conda environment.

Reviewed by @Spartee
@ashao ashao deleted the smartsim_site_build branch November 9, 2022 00:20
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.

3 participants