Skip to content
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

dace extension display error for access patterns and stack distance #240

Open
rohanrayan opened this issue Oct 11, 2023 · 2 comments
Open
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@rohanrayan
Copy link

rohanrayan commented Oct 11, 2023

Describe the bug
The extension doesn't animate matrix access for a simple use-case. Tested using

To Reproduce
Steps to reproduce the behavior:

  1. Generate the SDFG using the attached ipynb
  2. Open the generated SDFG using the VS-code extension
  3. Select the entire SDFG and click on the "Inspect access patterns (local view)" tab
  4. In the pop-ups for M and N, enter 10 in both
  5. In the local view tab, try moving the slider and you will see that only one input matrix access gets highlighted.
  6. Both Access pattern and Reuse distance only show up for one matrix for some reason.

Expected behavior
I expect both the input matrices to display the same access pattern in the extension.

dace version info
dace v0.14.4
VS code extension v1.5.9

vs_code_bug_reproduce.zip

@phschaad phschaad self-assigned this Oct 17, 2023
@phschaad phschaad added this to the October 2023 milestone Oct 17, 2023
@phschaad
Copy link
Collaborator

Thank you for reporting the issue.

For completeness, I am including the source that generated the SDFG, and a picture of the SDFG here.
Source program (simplified):

import dace

N = dace.symbol('N')
M = dace.symbol('M')

@dace.program(auto_optimize=True)
def madd(A: dace.float64[M, N], B: dace.float64[M, N]):
    for i in range(0, M):
        for j in range(0, N):
            A[i][j] += B[i][j]

SDFG:

image

Now, the reason the highlighting does not work as expected is because of this intermediate data container, __tmp0. Because of this, the accesses inferred from the computation do not happen to data container B, but rather the temporary container __tmp0. There are plans to change this behavior because it does not reflect the behavior when viewed on the entire map, as you were trying to do. I will bump the priority of that work item in response to this, since I'm glad to see the feature is being used ;-) I cannot promise that it will make it into the October patch of the extension though due to some other high priority items.

An additional note here, though, is that it seems like this is another manifestation of spcl/dace#1382 - since it seems like the __tmp0 container should not be there to begin with. @alexnick83 ?

@phschaad phschaad added the enhancement New feature or request label Oct 18, 2023
@alexnick83
Copy link

There isn't any issue with the container being there in the first place but I would expect it to be removed by strict transformations (before auto-optimization making the loop parallel).

@phschaad phschaad modified the milestones: October 2023, November 2023 Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants