[develop] Introduce InstanceRequirements schema and resource #5419
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add InstanceRequirementsDefinition class to hold all the attributes required to define a ComputeResource. Add a InstanceRequirementsComputeResource to model a ComputeResource defined through IntanceRequirements. Signed-off-by: Nicola Sirena <[email protected]>
NSsirena
force-pushed
the
wip/instance-requirements
branch
from
a27eb67 to
07aa96f
Compare
Add InstanceRequirementsSchema class to define the customer facing contract of the feature. Extends SlurmComputeResourceSchema to support InstanceRequirementsSchema definitions. Signed-off-by: Nicola Sirena <[email protected]>
Signed-off-by: Nicola Sirena <[email protected]>
Signed-off-by: Nicola Sirena <[email protected]>
A set of InstanceRequirements may return a long list of instance-types (or none) depening on the attributes, the region and the architecture. Signed-off-by: Nicola Sirena <[email protected]>
NSsirena
force-pushed
the
wip/instance-requirements
branch
from
07aa96f to
2a45dac
Compare
enrico-usai
reviewed
Jun 22, 2023
| @@ -8,6 +8,7 @@ CHANGELOG | |||
| - Allow configuration of static and dynamic node priorities in Slurm compute resources via the ParallelCluster configuration YAML file. | |||
| - Add support for Ubuntu 22. | |||
| - Allow memory-based scheduling when multiple instance types are specified for a Slurm Compute Resource. | |||
| - Allow definition of a Slurm Compute Resource through InstanceRequirements. | |||
There was a problem hiding this comment.
nit: InstanceRequirements
| } | ||
|
|
||
| response = self._client.get_instance_types_from_instance_requirements(config) | ||
| if "InstanceTypes" in response: |
There was a problem hiding this comment.
nit: you could write:
instance_types = response.get("InstanceTypes", [])
return [res.get("InstanceType") for res in instance_types]or:
return [instance_types.get("InstanceType") for instance_types in response.get("InstanceTypes", [])]| if self.accelerator_count > 0: | ||
| self.accelerator_types = ["gpu"] | ||
| self.accelerator_manufacturers = ["nvidia"] | ||
| else: |
There was a problem hiding this comment.
if you're forcing a specific behaviour you can avoid the set in the previous lines, that is useless:
self.accelerator_types = accelerator_types
self.accelerator_manufacturers = accelerator_manufacturers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Tests
References
Checklist