-
Notifications
You must be signed in to change notification settings - Fork 133
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
Fix array indirection to memlet subset promotion #1406
Conversation
31b15c3
to
8946323
Compare
tests/double_index_bug_test.py
Outdated
from dace import subsets | ||
|
||
|
||
def test_double_index_bug(): |
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.
Maybe I should put this test somewhere else? 🤔
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.
Probably in tests/passes/scalar_to_symbol_promotion_test.py?
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.
Looks good, see comment about test.
tests/double_index_bug_test.py
Outdated
from dace import subsets | ||
|
||
|
||
def test_double_index_bug(): |
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.
Probably in tests/passes/scalar_to_symbol_promotion_test.py?
tests/double_index_bug_test.py
Outdated
@@ -0,0 +1,48 @@ | |||
# Copyright 2019-2023 ETH Zurich and the DaCe authors. All rights reserved. |
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.
year
The current solution is rather hacky. I want to run the tests first to see the impacts of this change.
Additionally, there is no test yet, because validation doesn't catch the erroneous SDFG yet.
Overall, it's not clear currently how to solve the issue and the PR might change as we progress...