feat: set AcceleratorType for GPU sizes that lack it in the name - #43
Merged
Matthew Christopher (matthchr) merged 1 commit intoSep 2, 2026
Merged
Matthew Christopher (matthchr) merged 1 commit into
Matthew Christopher (matthchr) merged 1 commit into
Conversation
AcceleratorType is parsed out of the VM size name, so it is nil for the GPU sizes whose name has no accelerator part. Series alone does not identify the chip: ND96isr_H100_v5 and ND96isr_H200_v5 both give NDisr_v5, so Series + AcceleratorType is the pair that does, and it breaks when one of them is nil. The SKUs API cannot fill the gap. It exposes a GPU count but no model. Some family strings embed the chip, but not for these sizes, and the format is inconsistent across families, so parsing it would cover one of the affected sizes and add a second thing to keep working. Add a lookup consulted only when the regex finds no accelerator, so no size that already resolves can change. Covers 13 sizes over 6 series, values taken from the per-series pages on MS Learn. NVs_v2 is left out because its page is gone and no other source gives the chip. The lookup strips a _Promo suffix so promo variants resolve like the base size. There are none for these series today, but the regex already handles promo GPU sizes and the two should not disagree. Applied to both modules.
Matthew Christopher (matthchr)
approved these changes
Sep 2, 2026
This file contains hidden or 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
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.
Fixes #38
AcceleratorTypeis parsed out of the size name, so it comes back nil for GPU sizes whose name has no accelerator part. That matters becauseSeriesalone doesn't identify the chip:ND96isr_H100_v5andND96isr_H200_v5both giveNDisr_v5.Series+AcceleratorTypeis the pair that identifies a GPU, and it doesn't work when one of them is nil.The SKUs API doesn't have this data. I checked before writing the map (details in #38 (comment)).
What this adds: a lookup consulted only when the regex finds no accelerator, so no size that already resolves can change value.
The lookup strips a
_Promosuffix so promo variants resolve like the base size. There are none for these series today, but the regex already handles promo GPU sizes and the two shouldn't disagree.Applied to both modules. The map is duplicated in
vmsize.goandv2/vmsize.go, matching how the rest of the package is kept in sync.On the counts. 19 sizes have a
GPUscapability and a nilAcceleratorType. Three are NP-series, which the SKUs API reports underGPUsbut which are actually Xilinx FPGAs, so they're out of scope. That leaves the 16 GPU sizes from the issue.This covers 13 of them. The remaining 3 are
NV6s_v2,NV12s_v2andNV24s_v2: the NVv2 Learn page is gone, the retirement notice doesn't name the chip, and I found no other source that does, so I left them out rather than guess. Happy to add them if someone can confirm the GPU.