Precise dependencies compatibility #785
Draft
Conversation
This file contains 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
inducer
reviewed
Jun 20, 2023
inducer
force-pushed
the
precise-dependencies-compatibility
branch
from
7222048
to
8b96893
Compare
a-alveyblanc
force-pushed
the
precise-dependencies-compatibility
branch
from
8b96893
to
0cae494
Compare
|
@inducer This is ready for another look. |
inducer
force-pushed
the
precise-dependencies-compatibility
branch
5 times, most recently
from
bcb356a
to
47d3c01
Compare
inducer
force-pushed
the
precise-dependencies-compatibility
branch
from
47d3c01
to
5c84e95
Compare
inducer
reviewed
Aug 18, 2024
Comment on lines
+66
to
+91
| shared_inames_order_before = [ | ||
| domain_before.get_dim_name(dim_type.out, idim) | ||
| for idim in range(domain_before.dim(dim_type.out)) | ||
| if domain_before.get_dim_name(dim_type.out, idim) | ||
| in shared_inames] | ||
|
|
||
| shared_inames_order_after = [ | ||
| domain_after.get_dim_name(dim_type.out, idim) | ||
| for idim in range(domain_after.dim(dim_type.out)) | ||
| if domain_after.get_dim_name(dim_type.out, idim) | ||
| in shared_inames] | ||
|
|
||
| assert shared_inames_order_after == shared_inames_order_before | ||
| shared_inames_order = shared_inames_order_after |
There was a problem hiding this comment.
This snippet here would imply that the order of variables in a set is significant. This is not the case anywhere else in loopy, and I would prefer to not start having it be the case now. This behavior is also not documented.
There was a problem hiding this comment.
So... I tried, but I ran out of time. 🙂 There's a bit of algorithmic work to be done here, and some important algorithmic pieces are in #690. We can discuss in more detail during our meeting Monday.
Rather than hijack this PR for my big pile of unrelated changes, that's now #864, which this depends on. The relevant pieces from #690 are also in #864, so you'll be able to rely on them.
inducer
force-pushed
the
precise-dependencies-compatibility
branch
from
b475e5a
to
507792b
Compare
Closed
inducer
force-pushed
the
precise-dependencies-compatibility
branch
from
507792b
to
587f381
Compare
Merged
inducer
force-pushed
the
precise-dependencies-compatibility
branch
from
587f381
to
1a6548c
Compare
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.
HappensAfterdata structureloopy/kernel/dependency.py)InstructionBaseto accommodate precise dependency semantics (depends_on->happens_after)depends_onis nowhappens_afterNeeds:
UniqueNameGeneratorpytools#255 and a pytools release