kernels: append revision to metadata id to prevent collision

[sayakpaul]

Refer to https://huggingface.slack.com/archives/C090JN2P8NB/p1791497365338479.

If we load the same kernel from different revisions, the output modules aren't different.

from kernels import get_kernel

repo = "AntonV/dummy-rmsnorm-mlp-with-transformations-and-init"
old = get_kernel(repo, revision="d582b66bea8e567dd06e683eca611648cfe53a7b", trust_remote_code=True)
new = get_kernel(repo, revision="4a46a59cc7e02bf54a09f8c8a10e4ce209756029", trust_remote_code=True)

print("Old:", old.layers.__file__)
print("New:", new.layers.__file__)

assert old.layers is not new.layers, "different revisions loaded the same module"

(reproducer from @vasqu)

It happens because:

• First, we're registering a module with its metadata.id as the key in sys.modules:

  kernels/kernels/src/kernels/importer.py

  Line 88 in 2e4510a

  sys.modules[metadata.id] = module

  
  At this point, the package registration is done and the <metadata.id>.layers submodule (when available of course).
• Loading from a new revision creates a new package object and replaces sys.modules[metadata.id], but leaves the existing .layers entry.
• The new revision’s from . import layers reuses that cached submodule. Python caches imports by module name.

To avoid this collision, this PR proposes to add repo_info.revision to build the module key that would go into sys.modules.

[HuggingFaceDocBuilderDev]

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.

[vasqu]

Works for me locally at least 🫡 just 1 question and I guess Daniel or David need to review either way

[vasqu]

does repo info always include a revision, e.g. is a version resolved to a revision?

[sayakpaul]

Yup:

kernels/kernels/src/kernels/_versions.py

Lines 204 to 235 in 1bec2d3

	def resolve_kernel_version(repo_id: str, version: KernelVersion, *, local_files_only: bool) -> Oid:
	"""Resolve a kernel version to the commit it refers to.
	A `KernelVersion` can either be a version number or a revision (branch,
	tag, or commit). This function resolves the version or revision into a
	full Git commit SHA.
	"""
	if isinstance(version, KernelVersion.Version):
	# `name` rather than `ref`: the cache names its refs `v1`, not
	# `refs/heads/v1`.
	version_ref = resolve_version_spec_as_ref(repo_id, version.version, local_files_only=local_files_only)
	ref, commit = version_ref.name, Oid.from_str(version_ref.target_commit)
	elif isinstance(version, KernelVersion.Revision):
	ref, commit = version.revision, _resolve_ref(repo_id, version.revision, local_files_only=local_files_only)
	else:
	raise ValueError(f"Invalid version type: {version}")
	if not local_files_only:
	# Kernels are fetched by commit, since we need the commit hash for receipt
	# validation, etc. However, that means that snapshot downloads do not create
	# refs in the cache. This causes a kernel fetched by version/ref not to be
	# found in offline mode. To work around this problem, create a ref ourselves.
	#
	# Note that this can create the situation where the ref exists, but no
	# snapshot or an incomplete snapshot. However, this is fine for
	# huggingface_hub, since it also writes the ref before downloading the
	# snapshot:
	#
	# https://github.com/huggingface/huggingface_hub/blob/5a9cdda63f231a1b57a05eab88dc4357c790ba87/src/huggingface_hub/_snapshot_download.py#L426
	_record_ref_in_cache(repo_id, ref, str(commit))
	return commit

For local kernel, this is handled differently, i.e., no repo_info, of course.

[danieldk]

We shouldn't do this. The identifier from the metadata should be unique. This will still not solve it in contexts where we do not have a revision.

[sayakpaul]

But it doesn't seem to be 👀

[sayakpaul]

Closing because the kernel used in the reproducer doesn't follow the the requirements.

[github-actions]

Coverage report — kernels/

Measured on: Python 3.10 / Torch 2.13.0.
Other CI configurations are not included in this number.
Hardware-gated code paths (ROCm/XPU/NPU/Darwin/Windows) are excluded or unreachable on the Linux+CUDA runner.

Total coverage: 87.5% — threshold: 80% — ✅

Per-file breakdown

Name	Stmts	Miss	Cover	Missing
src/kernels/__init__.py	14	0	100%
src/kernels/_system.py	6	1	83%	10
src/kernels/_versions.py	130	14	89%	53, 59-60, 63-64, 102, 165-170, 199, 219
src/kernels/archs.py	56	1	98%	94
src/kernels/backends.py	214	62	71%	42, 46, 50-53, 70, 92, 110, 119, 123, 127-129, 150, 159, 163, 167-169, 190, 201, 203, 210-213, 222, 226, 230-250, 258, 281-301
src/kernels/compat.py	9	1	89%	5
src/kernels/deps.py	70	1	99%	56
src/kernels/hf_hub.py	63	2	97%	21, 23
src/kernels/importer.py	45	5	89%	80, 86, 89, 103-104
src/kernels/install.py	21	7	67%	76-100
src/kernels/layer/__init__.py	6	0	100%
src/kernels/layer/_interval_tree.py	103	4	96%	23, 52, 147, 150
src/kernels/layer/device.py	48	14	71%	42, 47-49, 91, 96-98, 101, 149, 152, 155-157
src/kernels/layer/func.py	85	6	93%	90, 115, 191, 311, 338, 368
src/kernels/layer/globals.py	5	0	100%
src/kernels/layer/kernelize.py	82	8	90%	259, 297, 305-306, 312, 316, 332-334
src/kernels/layer/layer.py	309	22	93%	211, 258, 285, 416, 437, 444-447, 453-454, 556-557, 578, 586, 597, 637, 641, 654, 708, 738, 813
src/kernels/layer/mode.py	14	0	100%
src/kernels/layer/repos.py	144	42	71%	27, 33, 36-43, 63-64, 70, 73-76, 90, 94, 103-104, 110, 113-116, 123-124, 130, 133-136, 143-144, 150, 153-156, 163-164, 170, 173-176, 257
src/kernels/load.py	71	2	97%	338, 378
src/kernels/locking.py	89	64	28%	35-83, 91-98, 102-125, 137, 152-159, 165-175, 179-186
src/kernels/python_deps.py	58	6	90%	59-60, 64-65, 101, 104
src/kernels/resolver.py	158	1	99%	231
src/kernels/status.py	50	2	96%	25, 79
src/kernels/validate.py	88	5	94%	9, 100, 167, 190-191
src/kernels/variants.py	278	17	94%	65, 96, 117, 147, 256-257, 299-302, 304, 388-394, 400-406, 455-461
src/kernels/verify.py	127	6	95%	46, 202-204, 318-319
TOTAL	2343	293	87%

Updated by the Test kernels workflow on commit 1bec2d37c3602a324cf8fe91e95b7c632921da1d.