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

Add tests for GPU accounting and fix parsing sinfo output #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nuno-silva
Copy link

These changes make GPU accounting work with slurm 19 (at least for my installation) and should help with testing different outputs in #38.

The sinfo parsing change is from #38 (comment).

Please let me know if I should add a copyright/licence header to the new files.

These changes make accounting work with slurm 19 and should help with vpenso#38
func ParseAllocatedGPUs() float64 {
var num_gpus = 0.0

func getSacctData() []byte {
args := []string{"-a", "-X", "--format=Allocgres", "--state=RUNNING", "--noheader", "--parsable2"}
Copy link

Choose a reason for hiding this comment

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

Note that in SLURM 20.11+ Allocgres is deprecated, consider changing to AllocTRES like in #34, or to make it version independent fist check if sacct --helpformat returns AllocGRES as a possible value.

Copy link
Author

Choose a reason for hiding this comment

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

I am aware of that. However, since I don't have a slurm v20 cluster, I'm not risking changing the code myself to make it compatible with that version. Instead, I'm adding tests for the current code which should make it easier to add support for slurm 20 later (e.g. in another PR).

@nuno-silva
Copy link
Author

friendly ping @vpenso @mtds

@nuno-silva
Copy link
Author

friendly ping @vpenso @mtds
should I target another branch, or maybe this is outdated after #38 (comment)? thanks

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.

2 participants