-
Notifications
You must be signed in to change notification settings - Fork 357
[Release] Bump version into 0.1.7.post1 #1506
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
Conversation
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
📝 WalkthroughWalkthroughThis pull request removes deprecated fast-math configuration handling across the codebase. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tilelang/engine/lower.py (1)
78-78: LGTM! Consistent removal of deprecated fast-math flag.The simplification to use only
TL_ENABLE_FAST_MATHaligns with the broader refactoring effort. Thebool()wrapper provides explicit type conversion, though it's functionally equivalent to the approach intilelang/jit/adapter/libgen.py(line 62) which omits the wrapper.For consistency, consider whether both locations should use the same pattern (either both with or both without
bool()), though this is a minor stylistic point.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
VERSIONtilelang/engine/lower.pytilelang/jit/adapter/libgen.pytilelang/transform/pass_config.py
💤 Files with no reviewable changes (1)
- tilelang/transform/pass_config.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.1 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
🔇 Additional comments (1)
tilelang/jit/adapter/libgen.py (1)
62-62: LGTM! Clean simplification of fast-math configuration.The removal of dual-flag handling (
TL_DISABLE_FAST_MATH/TL_ENABLE_FAST_MATH) in favor of a singleTL_ENABLE_FAST_MATHflag simplifies the configuration logic. The default value ofFalseis conservative and appropriate.
| @@ -1 +1 @@ | |||
| 0.1.7 | |||
| 0.1.7.post1 | |||
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Searching for TL_DISABLE_FAST_MATH references ==="
rg -n "TL_DISABLE_FAST_MATH" --type=py 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking git log for TL_DISABLE_FAST_MATH mentions ==="
git log --all --oneline --source --all | grep -i "TL_DISABLE_FAST_MATH\|fast_math\|deprecated" | head -20 2>/dev/null || echo "No git log matches"
echo -e "\n=== Searching for deprecation warnings related to fast_math ==="
rg -n -C3 "deprecat" --type=py | grep -i "fast\|math" 2>/dev/null || echo "No deprecation mentions found"
echo -e "\n=== Checking VERSION file history ==="
git log --all --oneline VERSION 2>/dev/null | head -10 || echo "Cannot access git history"Repository: tile-ai/tilelang
Length of output: 1231
🏁 Script executed:
#!/bin/bash
echo "=== Check PassConfigKey enum for TL_DISABLE_FAST_MATH ==="
rg -n "PassConfigKey|TL_DISABLE_FAST_MATH" --type=py -A2 | head -50
echo -e "\n=== Check git diff for recent VERSION bump ==="
git show be9d174 --stat
echo -e "\n=== Check recent ENABLE_FAST_MATH refactor ==="
git show e7e3835 --statRepository: tile-ai/tilelang
Length of output: 50372
🏁 Script executed:
#!/bin/bash
echo "=== Check the ENABLE_FAST_MATH refactor commit ==="
git show e7e3835 --name-only | head -30
echo -e "\n=== Check if TL_DISABLE_FAST_MATH enum value exists ==="
git show e7e3835:tilelang/transform/pass_config.py 2>/dev/null | grep -A5 -B5 "DISABLE_FAST_MATH\|ENABLE_FAST_MATH" || echo "Check previous version"
echo -e "\n=== Check current pass_config.py for these keys ==="
cat tilelang/transform/pass_config.py | grep -A2 -B2 "FAST_MATH\|ENABLE_FAST_MATH"Repository: tile-ai/tilelang
Length of output: 2743
Post-release versioning is inappropriate for removing public API.
According to semantic versioning best practices, deprecated APIs should first be marked in a minor release with warnings, and removal should only occur in a subsequent major release after users have time to migrate. The version bump to 0.1.7.post1 violates this practice.
Git history shows that TL_DISABLE_FAST_MATH was deprecated in commit e7e3835 (marked "will be deprecated in the 0.1.7 release"), but it has been completely removed from the PassConfigKey enum in this post-release. This is a breaking change because users still actively using TL_DISABLE_FAST_MATH (as seen in examples/deepseek_v32/inference/kernel.py) will encounter errors.
Post-release versions (.post1) are reserved for packaging or distribution fixes without functional changes. Removing a public configuration key warrants at least a minor version bump (e.g., 0.2.0). If the deprecation was properly introduced in 0.1.7 with runtime warnings, a subsequent minor or major release should have been used for the actual removal.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tilelang/engine/lower.py
🧰 Additional context used
🧬 Code graph analysis (1)
tilelang/engine/lower.py (1)
tilelang/transform/pass_config.py (1)
PassConfigKey(6-157)
| enable_fast_math = not disable_fast_math | ||
| else: | ||
| enable_fast_math = bool(cfg.get(PassConfigKey.TL_ENABLE_FAST_MATH, False)) | ||
| enable_fast_math = bool(cfg.get(PassConfigKey.TL_ENABLE_FAST_MATH, False)) |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to TL_DISABLE_FAST_MATH in the codebase
echo "=== Searching for TL_DISABLE_FAST_MATH references ==="
rg -n 'TL_DISABLE_FAST_MATH' --type=py
echo ""
echo "=== Searching in documentation ==="
rg -n 'TL_DISABLE_FAST_MATH' --type=md
echo ""
echo "=== Searching in all file types (case-insensitive) ==="
rg -in 'disable.*fast.*math' --max-count=20Repository: tile-ai/tilelang
Length of output: 575
🏁 Script executed:
#!/bin/bash
# Verify the context around line 78 in tilelang/engine/lower.py
echo "=== Context around line 78 in lower.py ==="
sed -n '70,110p' tilelang/engine/lower.py | cat -nRepository: tile-ai/tilelang
Length of output: 1927
🏁 Script executed:
#!/bin/bash
# Check if TL_ENABLE_FAST_MATH is properly defined/used elsewhere
echo "=== Searching for TL_ENABLE_FAST_MATH usage ==="
rg -n 'TL_ENABLE_FAST_MATH' --type=pyRepository: tile-ai/tilelang
Length of output: 10638
Critical issue: Incomplete deprecation of TL_DISABLE_FAST_MATH.
While the code change at line 78 is technically correct in using only TL_ENABLE_FAST_MATH with a default of False, the deprecation of TL_DISABLE_FAST_MATH is incomplete. The deprecated flag still exists and is actively used in the codebase:
examples/deepseek_v32/inference/kernel.py:11still usesTL_DISABLE_FAST_MATH: Truesrc/op/builtin.cc:27andsrc/op/builtin.h:47still define the C++ option
Breaking change: Code using the old TL_DISABLE_FAST_MATH flag will now silently fail because line 78 no longer reads this flag. This requires either:
- Updating all usages to
TL_ENABLE_FAST_MATH, or - Implementing a deprecation migration path that honors the old flag temporarily while warning users
as title.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.