Repository navigation
Revert "assert when found duplicated tuned shape" - #2502
Conversation
This reverts commit d3ed648.
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
|
There was a problem hiding this comment.
Pull request overview
Reverts the prior change that treated duplicated tuned shapes as a hard error during tuned-config CSV merges, returning to “drop duplicates and continue” behavior.
Changes:
- Reworked
update_config_files()merge behavior to sort byus, warn, and drop duplicate tuned rows instead of erroring. - Removed several model-specific untuned shape lists under
aiter/configs/model_configs/. - Added a few missing tuned GEMM entries for KimiK2 and DSV3.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| aiter/jit/core.py | Restores “warn + drop duplicates” merge behavior for tuned config files; simplifies merge input handling. |
| aiter/configs/model_configs/kimik2_fp4_untuned_fmoe.csv | Deleted model-specific untuned FMoE shape list. |
| aiter/configs/model_configs/kimik2_bf16_untuned_gemm.csv | Deleted model-specific untuned BF16 GEMM shape list. |
| aiter/configs/model_configs/kimik2_bf16_tuned_gemm.csv | Added a tuned GEMM row for (M=80, N=2112, K=7168). |
| aiter/configs/model_configs/gptoss_bf16_untuned_gemm.csv | Deleted model-specific untuned BF16 GEMM shape list. |
| aiter/configs/model_configs/dsv3_fp4_untuned_fmoe.csv | Deleted model-specific untuned FMoE shape list. |
| aiter/configs/model_configs/dsv3_bf16_untuned_gemm.csv | Deleted model-specific untuned BF16 GEMM shape list. |
| aiter/configs/model_configs/dsv3_bf16_tuned_gemm.csv | Added several tuned GEMM rows for N=2112 at multiple M values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| merge_df = pd.concat([df for _, df in source_pairs], ignore_index=True) | ||
| df_list.append(df) | ||
| else: | ||
| logger.info(f"path {i+1}: {path} (not exist)") |
There was a problem hiding this comment.
The missing-path log uses enumerate(path_list[1:]) but formats the index as i+1, so it reports the second path as "path 1". If you want the log index to correspond to the original path_list position, this should be offset by 2 (or log the actual position/index directly).
| logger.info(f"path {i+1}: {path} (not exist)") | |
| logger.info(f"path {i+2}: {path} (not exist)") |
| df_list.append(pd.read_csv(path_list[0])) | ||
| for i, path in enumerate(path_list[1:]): |
There was a problem hiding this comment.
pd.read_csv(path_list[0]) is executed unconditionally. If the first path in the env var / merged list doesn’t exist, this will raise a pandas FileNotFoundError before you can skip missing paths (and it bypasses the previous, clearer FileNotFoundError when no paths exist). Consider iterating over all paths with an existence check (including index 0) and raising a controlled error if none exist.
| assert ( | ||
| base_cols == new_cols | ||
| ), f"Column mismatch between {path_list[0]} and {path}, {base_cols}, {new_cols}" |
There was a problem hiding this comment.
Column mismatch detection uses assert. Asserts are stripped with Python optimizations (-O), which would silently skip this validation. Use an explicit exception (e.g., ValueError) instead so mismatched CSV schemas are always caught with a stable error type.
| assert ( | |
| base_cols == new_cols | |
| ), f"Column mismatch between {path_list[0]} and {path}, {base_cols}, {new_cols}" | |
| if base_cols != new_cols: | |
| raise ValueError( | |
| f"Column mismatch between {path_list[0]} and {path}, {base_cols}, {new_cols}" | |
| ) |
| else: | ||
| logger.info(f"path {i+1}: {path} (not exist)") | ||
| merge_df = ( | ||
| pd.concat([df for df in df_list if not df.empty], ignore_index=True) |
There was a problem hiding this comment.
pd.concat([df for df in df_list if not df.empty], ...) can raise ValueError: No objects to concatenate when all loaded CSVs are empty (common for files that only contain headers). Previously concatenation included empty dataframes and produced an empty merge_df. Handle the “all empty” case explicitly (or avoid filtering empties) to prevent a runtime crash.
| pd.concat([df for df in df_list if not df.empty], ignore_index=True) | |
| pd.concat(df_list, ignore_index=True) |
* Reapply "assert when found duplicated tuned shape (#2376)" (#2502) This reverts commit 46afd48. * update blockscale_bpreshuffle_gemm * Update aiter/configs/model_configs/dsv3_bf16_untuned_gemm.csv Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * add fused_qk_norm_group_quant kernel * update untuned bf16 gemm csv * remove unrelated non-core changes from duplicate-shape PR Keep PR #2503 focused on duplicate-shape handling in aiter/jit/core.py and model-config CSV updates by reverting unintentionally included fused_qk/allreduce/test changes. Made-with: Cursor * revert remaining non-csv files from PR #2503 Keep the PR scope limited to aiter/jit/core.py and model config CSV changes by removing the remaining communication and multigpu test file deltas. Made-with: Cursor * revert hook-only non-csv churn from PR #2503 Drop remaining non-core, non-csv diffs introduced by hook normalization so the PR scope stays limited to aiter/jit/core.py and model config CSV changes. Made-with: Cursor --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Lingpeng Jin <103567126+valarLip@users.noreply.github.com>
Reverts #2376