[Lang] Make qd.static not bypass the pure-kernel purity check#733
Open
hughperkins wants to merge 9 commits into
Open
[Lang] Make qd.static not bypass the pure-kernel purity check#733hughperkins wants to merge 9 commits into
hughperkins wants to merge 9 commits into
Conversation
A captured module global wrapped in qd.static(...) previously skipped the [PURE.VIOLATION] check because both purity gates were guarded by `not is_in_static_scope`. That is unsound for fastcache: qd.static only controls compile-time evaluation and never puts the value into the cache key, so a static-wrapped global is still a purity violation (stale-cache hazard). Remove the static-scope exemption from both gates (build_Name and build_Attribute). Legitimate static metaprogramming over parameters/locals is unaffected, since those never set violates_pure. Document in fastcache.md that qd.static is not a purity escape hatch.
A captured string only affects a fastcache kernel through compile-time qd.static branches, but its value never enters the cache key, so it is cache-unsafe in the same way as a captured int/float. Add str to the flagged types in build_Name and build_Attribute, and document it in fastcache.md. Update the tile and src_ll_cache tests that captured globals (bools, strings) inside qd.static in pure kernels to pass those values as template parameters so the values enter the cache key.
…ion) During a transition period, a captured global accessed inside a qd.static scope of a pure kernel now emits a warning rather than raising a QuadrantsCompilationError, giving downstream code time to migrate such constants to kernel parameters. Direct captured-global access (not wrapped in qd.static) still raises. Adds a test covering both the captured-name and captured-attribute paths, and updates the fastcache user guide.
…erage, comment width - test_pure_validation_static_scope_warns: restrict to a single (CPU) arch. The purity check is an arch-independent AST analysis; running it across multiple archs in one xdist worker let a fastcache hit from one arch suppress the warning on the next, making pytest.warns flaky on Mac (arm64 after vulkan). - Add test_pure_validation_str covering the captured-str purity violation (str branch was previously untested). - Wrap two transition-period comments to <=120 chars (drop the em-dash).
Collaborator
Author
|
running Genesis benchmarks and unit tests |
Collaborator
Author
|
Note: next step for this is to run genesis benchmarks and unit tests with warnings tno converted to errors for static, and see how the output looks like |
Collaborator
Author
|
running Genesis enhmarks without warnings becoming errors: |
Collaborator
Author
|
@codex review |
Collaborator
Author
|
Genesis benchmarks run, in conjunction with Genesis-Embodied-AI/genesis-world#3034 : 
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84b9ce4aba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # ``str`` is included alongside the numeric/``Field`` types: a captured string only affects a kernel through | ||
| # compile-time ``qd.static`` branches, and its value never enters the fastcache key, so it is cache-unsafe | ||
| # in exactly the same way as a captured int/float. | ||
| if isinstance(node.ptr, (float, int, str, Field)): |
There was a problem hiding this comment.
When a fastcache kernel uses a captured container in qd.static, e.g. choices = [1, 2] followed by if qd.static(choices[idx]), the choices Name is marked violates_pure but its value is a list/dict/tuple, so this type guard skips the warning/error; the later Subscript only propagates the flag and does not enforce it. Because those container contents still are not included in the fastcache key, changing them can silently reuse stale compiled code even though the new docs say qd.static-wrapped captured globals are flagged.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A captured module global wrapped in qd.static(...) previously skipped the
[PURE.VIOLATION] check because both purity gates were guarded by
not is_in_static_scope. That is unsound for fastcache: qd.static onlycontrols compile-time evaluation and never puts the value into the cache key,
so a static-wrapped global is still a purity violation (stale-cache hazard).
Remove the static-scope exemption from both gates (build_Name and
build_Attribute). Legitimate static metaprogramming over parameters/locals is
unaffected, since those never set violates_pure. Document in fastcache.md that
qd.static is not a purity escape hatch.Issue: #
Brief Summary
copilot:summary
Walkthrough
copilot:walkthrough