-
Notifications
You must be signed in to change notification settings - Fork 584
misc: Update artifacts docstring and MetaInfoHash #1967
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,10 @@ | |
| from typing import List | ||
|
|
||
| from . import env as jit_env | ||
| from ..artifacts import ArtifactPath, MetaInfoHash | ||
| from ..artifacts import ArtifactPath, CheckSumHash | ||
| from .core import JitSpec, gen_jit_spec, current_compilation_context, sm90a_nvcc_flags | ||
| from .cpp_ext import is_cuda_version_at_least | ||
| from .cubin_loader import get_cubin | ||
| from .cubin_loader import get_cubin, get_meta_hash | ||
| from .gemm.cutlass.generate_kernels import generate_gemm_operations | ||
|
|
||
|
|
||
|
|
@@ -180,10 +180,16 @@ def gen_trtllm_gen_fused_moe_sm100_module() -> JitSpec: | |
| include_path = f"{ArtifactPath.TRTLLM_GEN_BMM}/include" | ||
| header_name = "flashinferMetaInfo" | ||
|
|
||
| # Check if checksums.txt exists in the cubin directory | ||
| checksum_path = f"{ArtifactPath.TRTLLM_GEN_BMM}/checksums.txt" | ||
| checksum = get_cubin(checksum_path, CheckSumHash.TRTLLM_GEN_BMM) | ||
| assert checksum, f"Failed to get checksums.txt from {checksum_path}" | ||
| meta_hash = get_meta_hash(checksum) | ||
|
|
||
| # use `get_cubin` to get "flashinferMetaInfo.h" | ||
| metainfo = get_cubin( | ||
| f"{include_path}/{header_name}.h", | ||
| MetaInfoHash.TRTLLM_GEN_BMM, | ||
| meta_hash, | ||
| ) | ||
|
Comment on lines
+183
to
193
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion | π Major π§© Analysis chainChecksum β MetaInfo flow is correct; ensure name parity across modules. Logic is good, but depends on get_meta_hash correctly matching MetaInfo and consistent header_name across files. After fixing get_meta_hash and aligning header_name in attention, this path should be stable. To verify naming consistency across the repo: π Script executed: #!/bin/bash
# Find all header_name assignments and their values
rg -n -C1 -P 'header_name\s*=\s*["\']flash(?:infer|Infer)MetaInfo["\']'Length of output: 158 π Script executed: #!/bin/bash
# Search for header_name assignments across Python files
rg -n -C2 -e 'header_name\s*=\s*' --type py
# Search for MetaInfo occurrences across Python files
rg -n 'MetaInfo' --type pyLength of output: 4624 Unify
|
||
| # make sure "flashinferMetaInfo.h" is downloaded or cached | ||
| assert metainfo, f"{header_name}.h not found" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_meta_hash: match MetaInfo header precisely; handle decoding/split robustly.
Current logic returns the first ".h" sha, which can select the wrong file and break header fetch. Also may ValueError on malformed lines and assume UTFβ8 without fallback.
Apply:
π Committable suggestion
π§° Tools
πͺ Ruff (0.14.1)
148-148: Avoid specifying long messages outside the exception class
(TRY003)
π€ Prompt for AI Agents