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

update slurm.conf #134

Merged
merged 1 commit into from
Jun 27, 2023
Merged

update slurm.conf #134

merged 1 commit into from
Jun 27, 2023

Conversation

andriy-safe-ai
Copy link
Contributor

@andriy-safe-ai andriy-safe-ai commented Jun 26, 2023

Add slurm config from prod to source.

Deployed a cpu dev cluster with the updated template.

@andriy-safe-ai andriy-safe-ai self-assigned this Jun 26, 2023
@andriy-safe-ai andriy-safe-ai linked an issue Jun 26, 2023 that may be closed by this pull request
Copy link
Contributor

@steven-safeai steven-safeai left a comment

Choose a reason for hiding this comment

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

Double check that the keys don't already exist such as the ones I pointed out.

Also link to the Github issues. I think I have it as one or more issues that this PR is trying to address.

@@ -35,20 +36,49 @@ SelectTypeParameters=CR_Core
AccountingStorageHost={{ bastion[0] }}
AccountingStorageType=accounting_storage/slurmdbd
TaskPlugin=task/affinity,task/cgroup
AccountingStorageTRES=gres/gpu,gres/gpu:A100
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is at the bottom of this file please edit it there or double check.

Copy link
Contributor Author

@andriy-safe-ai andriy-safe-ai Jun 26, 2023

Choose a reason for hiding this comment

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

@steven-basart
I just checked slurm.conf in prod and this is where it is located.

Copy link
Contributor

@steven-safeai steven-safeai Jun 26, 2023

Choose a reason for hiding this comment

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

I'm saying this file has these variables already defined below. Please see comment marked

Copy link
Contributor

Choose a reason for hiding this comment

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

If you wish to move them here to be consistent with prod that's fine but we can't doubly define the variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@steven-basart
Ok. I see it now.

Copy link
Contributor

@steven-safeai steven-safeai left a comment

Choose a reason for hiding this comment

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

Please fix

@@ -35,20 +36,49 @@ SelectTypeParameters=CR_Core
AccountingStorageHost={{ bastion[0] }}
AccountingStorageType=accounting_storage/slurmdbd
TaskPlugin=task/affinity,task/cgroup
AccountingStorageTRES=gres/gpu,gres/gpu:A100
Copy link
Contributor

@steven-safeai steven-safeai Jun 26, 2023

Choose a reason for hiding this comment

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

I'm saying this file has these variables already defined below. Please see comment marked

Copy link
Contributor

@steven-safeai steven-safeai left a comment

Choose a reason for hiding this comment

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

LGTM

@andriy-safe-ai andriy-safe-ai force-pushed the 130-update-slurm-configuration branch from f626f26 to 16f3008 Compare June 27, 2023 17:31
@andriy-safe-ai andriy-safe-ai merged commit 74f3c85 into main Jun 27, 2023
@andriy-safe-ai andriy-safe-ai deleted the 130-update-slurm-configuration branch June 27, 2023 17:32
andriy-safe-ai pushed a commit that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants