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

WX-1810 WX-1830 n1/n2/n2d machine types, cpuPlatform on GCPBATCH #7518

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

mcovarr
Copy link
Contributor

@mcovarr mcovarr commented Aug 27, 2024

Description

Uses GcpBatchMachineConstraints#machineType to choose the appropriate machine type for GCP Batch.

Release Notes Confirmation

CHANGELOG.md

  • I updated CHANGELOG.md in this PR
  • I assert that this change shouldn't be included in CHANGELOG.md because it doesn't impact community users

Terra Release Notes

  • I added a suggested release notes entry in this Jira ticket
  • I assert that this change doesn't need Jira release notes because it doesn't impact Terra users

@mcovarr mcovarr changed the title WX-1830 Demo non-workingness of cpuPlatform on GCPBATCH WX-1810 WX-1830 n1/n2/n2d machine types, cpuPlatform on GCPBATCH Aug 27, 2024
@mcovarr mcovarr marked this pull request as ready for review August 27, 2024 21:04
@mcovarr mcovarr requested a review from a team as a code owner August 27, 2024 21:04
@mcovarr mcovarr requested a review from AlexITC August 27, 2024 21:04
googleLegacyMachineSelection = false,
jobLogger = jobLogger
)
val instancePolicy = createInstancePolicy(cpuPlatform, spotModel, accelerators, allDisks, machineType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using named arguments to avoid confusing cpuPlatform/machineType because both are strings.

// Similarly, CPU platform of AMD Rome corresponds to the machine type n2d.
val customMachineType =
cpuPlatformOption match {
case Some(GcpBatchRuntimeAttributes.CpuPlatformIntelCascadeLakeValue) => N2CustomMachineType
case Some(GcpBatchRuntimeAttributes.CpuPlatformIntelIceLakeValue) => N2CustomMachineType
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can cover this with tests at GcpBatchMachineConstraintsSpec

@mcovarr mcovarr merged commit 985ccf5 into develop Aug 28, 2024
39 checks passed
@mcovarr mcovarr deleted the wx_1830_cpu_platform_batch branch August 28, 2024 13:29
@mcovarr
Copy link
Contributor Author

mcovarr commented Aug 30, 2024

Addreses #7474

ovesh pushed a commit to deepgenomics/cromwell that referenced this pull request Oct 10, 2024
…e backend (#1)

* WX-1810 WX-1830 n1/n2/n2d machine types, cpuPlatform on GCPBATCH (broadinstitute#7518)
* feat: [GCP Batch] Support passing standard machine types to the Google backend

---------

Co-authored-by: Miguel Covarrubias <mcovarr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants