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

[bug] Fix the mapping from virtual axes to physical axes again #3170

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

strongoier
Copy link
Contributor

@strongoier strongoier commented Oct 12, 2021

Related issue = #3154

In Taichi docs, there is a subsection introducing row-major vs column-major dense layouts:

### Row-major versus column-major

However, in Taichi v0.8.1,

ti.root.dense(ti.j, 2).dense(ti.i, 3).place(x)

is equivalent to

ti.root.dense(ti.i, 2).dense(ti.j, 3).place(x)

The reason is that virtual axes (user-view) are set according to the appearance order in SNode definition of physical axes (ti.i, ti.j, ... in the physical world). In the first example, the first axis in user-view is ti.j, and the second axis is ti.i, while in the second example, the first axis is ti.i, and the second axis is ti.j. Now comes the tricky thing - as for the memory layouts, in the first example we split ti.j first, and in the second example we split ti.i first. That is, the major axes in layouts are the same as the first axes in user-view. So essentially there is no user-viable difference between these two examples, which breaks the docs.

This PR, together with #3159, fixes this issue by setting virtual axes according to the alphabetical order of physical axes. Now in both examples, the first axis in user-view is ti.i, and the second axis in user-view is ti.j. Then the column-major layout is finally created in the first example.

@netlify
Copy link

netlify bot commented Oct 12, 2021

✔️ Deploy Preview for jovial-fermat-aa59dc canceled.

🔨 Explore the source changes: 51106ce

🔍 Inspect the deploy log: https://app.netlify.com/sites/jovial-fermat-aa59dc/deploys/616564bb280dc60008068972

Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Would appreciate if this bug could be described better though..

@strongoier
Copy link
Contributor Author

LGTM! Would appreciate if this bug could be described better though..

The description has just been updated.

@k-ye k-ye merged commit 43252f7 into taichi-dev:master Oct 12, 2021
@strongoier strongoier deleted the fix-index-mapping-2 branch October 22, 2021 08:17
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