Fix red arg to subst#1016
Merged
Merged
Conversation
cb7187c to
b83ddf3
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors reduction_arg_to_subst_rule to use Loopy’s rule-aware mapping machinery (with stack matching via within) so that reduction arguments can be rewritten more robustly, alongside a broader set of typing/documentation cleanups (notably around instruction IDs and barrier/no-sync kinds).
Changes:
- Reworked
reduction_arg_to_subst_ruleto use aRuleAwareIdentityMapper-based mapper withwithinstack matching. - Introduced/propagated stronger typing for instruction IDs (
InsnId), barrier kinds (BarrierKind), and no-sync scopes (NoSyncScope) across transforms/scheduling/instruction definitions. - Updated Sphinx documentation/config to reference the new type aliases.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
loopy/typing.py |
Adds InsnId to the public typing docs. |
loopy/transform/data.py |
Refactors reduction_arg_to_subst_rule to rule-aware mapping + adds typing. |
loopy/transform/add_barrier.py |
Tightens typing of barrier kind arguments using BarrierKind. |
loopy/symbolic.py |
Fixes reduction callback typing to use Reduction instead of Expression. |
loopy/schedule/__init__.py |
Propagates barrier/no-sync typing and adds more explicit type annotations. |
loopy/kernel/instruction.py |
Introduces NoSyncScope, BarrierKind, and to_barrier_kind; tightens no_sync_with typing. |
loopy/kernel/creation.py |
Uses to_barrier_kind when creating barrier instructions from textual syntax. |
loopy/kernel/array.py |
Narrows ArrayBase.offset type annotation (currently inconsistent with runtime behavior). |
loopy/kernel/__init__.py |
Tightens typing for no-sync scope queries and adds type annotations. |
doc/ref_kernel.rst |
Attempts to document new type aliases (currently using autoclass). |
doc/conf.py |
Adds missing-reference aliases for the new type aliases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b83ddf3 to
3400269
Compare
3400269 to
e39e493
Compare
Owner
Author
|
All better now. This was badly broken previously. |
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
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.
@majosm This became much bigger than I had hoped. I'd like it if you could take a quick look at the changes in
reduction_arg_to_subst_rule.