Skip to content

kernels: add a function system_variants#423

Open
danieldk wants to merge 2 commits intomainfrom
system-variants
Open

kernels: add a function system_variants#423
danieldk wants to merge 2 commits intomainfrom
system-variants

Conversation

@danieldk
Copy link
Copy Markdown
Member

@danieldk danieldk commented Apr 1, 2026

This function lists all the variants that are available on the current system.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

This function lists all the variants that are available on the current
system.

version: Version | None
_VARIANT_REGEX: ClassVar[re.Pattern] = re.compile(
r"torch(\d+?)(\d+)(?:-(cxx11|cxx98))?"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Prepare for variants without an ABI tag (see extra variants in tests).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here I think one liner explainer comment would be nice. Bits around (cxx11|cxx98) aren't particularly clear I think.


version: Version | None
_VARIANT_REGEX: ClassVar[re.Pattern] = re.compile(
r"torch(\d+?)(\d+)(?:-(cxx11|cxx98))?"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here I think one liner explainer comment would be nice. Bits around (cxx11|cxx98) aren't particularly clear I think.

cxx11_abi = torch.compiled_with_cxx11_abi()
return [
Torch(version=torch_version, cxx11_abi=cxx11_abi),
Torch(version=torch_version, cxx11_abi=None),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious why include Torch(version=torch_version, cxx11_abi=None) here as well?

Comment on lines +204 to +212
def possible_variants() -> list["ArchVariant"]:
frameworks: list[Torch | TvmFfi] = (
Torch.possible_variants() + TvmFfi.possible_variants()
)
archs = Arch.possible_variants()
return [
ArchVariant(framework=fw, arch=arch)
for fw, arch in itertools.product(frameworks, archs)
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is so neat! As declarative as it gets.

Comment on lines +262 to +280
if parts[0] == "torch":
# noarch: e.g. "torch-cpu"
return NoarchVariant(
framework=TorchNoarch(), arch=Noarch.parse("-".join(parts[1:]))
)
elif parts[0].startswith("torch"):
if len(parts) >= 2 and parts[1] in ("cxx11", "cxx98"):
framework_str = f"{parts[0]}-{parts[1]}"
arch_parts = parts[2:]
else:
framework_str = parts[0]
arch_parts = parts[1:]
return ArchVariant(
framework=Torch.parse(framework_str), arch=Arch.parse(arch_parts)
)
elif parts[0] == "tvm" and len(parts) >= 2 and parts[1].startswith("ffi"):
return ArchVariant(
framework=TvmFfi.parse(f"tvm-{parts[1]}"), arch=Arch.parse(parts[2:])
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This conditional logic feels a bit rigid, but I guess it's purely heuristics-driven and isn't meant to change? If it changes then there's something wrong.

framework = TvmFfi.parse(f"tvm-{parts[1]}")
arch = Arch.parse(parts[2:])
else:
raise ValueError(f"Unknown framework in variant string: {variant_str!r}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, we're removing parse() from the variant dataclasses and keeping a central parse_variant() utility?

"""Parse a variant string into an ArchVariant or NoarchVariant."""
parts = variant_str.split("-")

if parts[0] == "torch":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should there be a constraint on the length of parts in case of NoarchVariant because ArchVariant will also have parts[0] == "torch"?

system_variants,
)

VARIANT_STRINGS = [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It could be nice to programatically construct this list against a predefined set of framework (+ versions), platform, arch, backend, etc.

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.

3 participants