Skip to content

[graphics] Fix for canonicalizeHighlights#34

Open
gilbertmike wants to merge 1 commit intoFibertree-Project:masterfrom
gilbertmike:master
Open

[graphics] Fix for canonicalizeHighlights#34
gilbertmike wants to merge 1 commit intoFibertree-Project:masterfrom
gilbertmike:master

Conversation

@gilbertmike
Copy link

The method canonicalizeHighlights seems to add an extraneous level of list nesting, causing errors.

Previously:

# This formatting goes through code path highlights.py:332, but should not have. But it silently works.
canvas.addFrame((m,k), (k,n), (m,n))
# This does not work because of the additional level of nesting added in tensor_canvas.py:301
canvas.addFrame([(m,k)], [(k,n)], [(m,n)])

With the change, both works and the different formats follow the correct code path.

@nandeeka
Copy link
Collaborator

Hi Michael,
Your change does not support flattened ranks. Here is an example of a kernel that would not work with your change:

K = 4
M = 5
density = [0.9, 0.5]
seed = 0

A_KM = Tensor.fromRandom(rank_ids=["K", "M"], shape=[K, M], seed=seed, density=density, name="A")
B_KM = Tensor.fromRandom(rank_ids=["K", "M"], shape=[K, M], seed=seed + 1, density=density, name="B")

A_KM_flat = A_KM.flattenRanks(depth=0, levels=1, coord_style="tuple")
A_KM_flat.setRankIds(rank_ids=["KM"])
B_KM_flat = B_KM.flattenRanks(depth=0, levels=1, coord_style="tuple")
B_KM_flat.setRankIds(rank_ids=["KM"])

Z_KM_flat = Tensor(rank_ids=["KM"], name="Z")

a_km = A_KM_flat.getRoot()
b_km = B_KM_flat.getRoot()
z_km = Z_KM_flat.getRoot()

canvas = createCanvas(A_KM_flat, B_KM_flat, Z_KM_flat)
for (k, m), (z_ref, (a_val, b_val)) in z_km << (a_km & b_km):
    z_ref <<= a_val * b_val
    canvas.addActivity(((k, m),), ((k, m),), ((k, m),))
Z_KM = Z_KM_flat.unflattenRanks(depth=0, levels=1)
Z_KM.setRankIds(rank_ids=["K", "M"])

displayCanvas(canvas)

I do not want to break the ability to display flattened ranks because they are used heavily by the accelerator zoo (see https://github.com/FPSG-UIUC/accelerator-zoo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants