Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Specify node feature for slurm job #529

Merged
merged 10 commits into from
Mar 22, 2024
Merged

Specify node feature for slurm job #529

merged 10 commits into from
Mar 22, 2024

Conversation

amandarichardsonn
Copy link
Contributor

@amandarichardsonn amandarichardsonn commented Mar 21, 2024 •
edited
Loading

This PR adds the method set_node_feature to srunSettings that accepts a str or list of strs. Users may now specify node constraints for slurm jobs.

@amandarichardsonn amandarichardsonn marked this pull request as draft March 21, 2024 23:11
@codecov Codecov
Copy link

codecov bot commented Mar 21, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.72%. Comparing base (06d6166) to head (60888e4).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #529   +/-   ##
========================================
  Coverage    90.72%   90.72%           
========================================
  Files           60       60           
  Lines         3816     3818    +2     
========================================
+ Hits          3462     3464    +2     
  Misses         354      354
Files Coverage Δ
smartsim/settings/base.py 98.20% <100.00%> (+0.02%) ⬆️

@amandarichardsonn amandarichardsonn marked this pull request as ready for review March 22, 2024 04:24
@amandarichardsonn amandarichardsonn changed the title draft Add method to specify node features for slurm job Mar 22, 2024
@amandarichardsonn amandarichardsonn changed the title Add method to specify node features for slurm job Specify node feature for slurm job Mar 22, 2024
al-rigazzi
al-rigazzi requested changes Mar 22, 2024
View reviewed changes
Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

A couple of minor details to fix for consistency reasons, mostly

al-rigazzi
al-rigazzi requested changes Mar 22, 2024
View reviewed changes
Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

One super-minor detail to go!

al-rigazzi
al-rigazzi approved these changes Mar 22, 2024
View reviewed changes
Copy link
Collaborator

@al-rigazzi al-rigazzi 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 fixing the last details!

@amandarichardsonn amandarichardsonn merged commit 4b35cc9 into CrayLabs:develop Mar 22, 2024
44 checks passed
@amandarichardsonn amandarichardsonn deleted the node_features branch March 22, 2024 22:26
@amandarichardsonn amandarichardsonn added area: launcher Issues related to any of the launchers within SmartSim type: feature Issues that include feature request or feature idea labels Apr 25, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@al-rigazzi al-rigazzi al-rigazzi approved these changes

Assignees
No one assigned
Labels
area: launcher Issues related to any of the launchers within SmartSim type: feature Issues that include feature request or feature idea
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amandarichardsonn @al-rigazzi