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

Fix typos in cute docs #1486

Merged
merged 2 commits into from
May 2, 2024
Merged

Fix typos in cute docs #1486

merged 2 commits into from
May 2, 2024

Conversation

irasin
Copy link
Contributor

@irasin irasin commented Apr 16, 2024

No description provided.

@ccecka
Copy link

ccecka commented Apr 16, 2024

Thanks!

@irasin
Copy link
Contributor Author

irasin commented Apr 16, 2024

Hi, @ccecka, in this example, I think it should be <3:3, (2,4):(1,8)> instead of <3:4, (2,4):(1,8)> , but I can't modify the image.
https://github.com/NVIDIA/cutlass/blob/main/media/images/cute/divide2.png

@ccecka
Copy link

ccecka commented Apr 16, 2024

You're right about that one too, thanks!

Here's the edited image to include in this MR so we can keep everything together.

divide2

@ccecka
Copy link

ccecka commented Apr 16, 2024

Looks like that last line describing divide2.png also has an error.

It should read

Note that the first mode of each mode of the result is the sublayout (3,(2,4)):(177,(13,2)) and is precisely the result we would have received if we had applied composition instead of logical_divide.

@irasin
Copy link
Contributor Author

irasin commented Apr 17, 2024

Found another mistake in https://github.com/NVIDIA/cutlass/blob/main/media/docs/cute/03_tensor.md#owning-tensors

The example of make_tensor_like should be

Tensor rmem_4x8_pad = make_tensor<float>(Shape <_4, _8>{},
                                         Stride<_2,_32>{});
Tensor rmem_4x8_like = make_tensor_like(rmem_4x8_pad);

and according to the order of the Stride<_2,_32>{}, the printed output should be

rmem_4x8_pad  : ptr[32b](0x7ff1c8fff920) o (_4,_8):(_2,_32)
rmem_4x8_like : ptr[32b](0x7f4158fffc60) o (_4,_8):(_1,_4)

@ccecka
Copy link

ccecka commented Apr 17, 2024

Gah, good finds. Can you edit those to:

Tensor rmem_4x8_pad = make_tensor<float>(Shape < _4,_8>{},
                                         Stride<_32,_2>{});
Tensor rmem_4x8_like = make_tensor_like(rmem_4x8_pad);

and

rmem_4x8_pad  : ptr[32b](0x7ff1c8fff920) o (_4,_8):(_32,_2)
rmem_4x8_like : ptr[32b](0x7f4158fffc60) o (_4,_8):(_8,_1)

Thanks for your help!

@irasin
Copy link
Contributor Author

irasin commented Apr 17, 2024

Got it. I am a beginner in cutlass/cute. I will read all cute docs, fix the typos and submit them in this MR.

@irasin irasin changed the title fix typos in 02_layout_algebra.md Fix typos in cute docs Apr 17, 2024
@irasin irasin force-pushed the main branch 2 times, most recently from 3f61d6b to 448b0df Compare April 18, 2024 11:44
@ccecka
Copy link

ccecka commented Apr 18, 2024

EDIT: Let's actually keep the references to and uses of composition(Tensor, Layout) the way they are and just add an implementation for completeness purposes.

@ccecka
Copy link

ccecka commented Apr 18, 2024

For explanation purposes, I would rather keep it as the simple composition. There are other examples that use a *_divide.

@hwu36
Copy link
Collaborator

hwu36 commented Apr 24, 2024

@ccecka , @thakkarV , more work needed in this pr?

@ccecka
Copy link

ccecka commented Apr 24, 2024

@irasin Are you happy with this PR?

This is a good time to add in the composition(Tensor, Layout) to tensor.hpp?

@irasin
Copy link
Contributor Author

irasin commented Apr 25, 2024

Sorry for the lack of updates.
Since I have some new work recently, I have to temporarily suspend my study of cute.
Please review and merge this PR if everything's OK.

@ccecka , About the composition(Tensor, Layout), I don't know what your expected implementation should be. Maybe you can add it in someother PR?

@ccecka
Copy link

ccecka commented Apr 25, 2024

Ok @hwu36, let's merge this now and I'll deal with the missing function in another review.

Thanks for your help!

@hwu36 hwu36 merged commit acc3ee1 into NVIDIA:main May 2, 2024
aacostadiaz pushed a commit to codeplaysoftware/cutlass-fork that referenced this pull request Jul 16, 2024
* fix typos in 02_layout_algebra.md

* fix typos in 03_tensor.md
aacostadiaz pushed a commit to codeplaysoftware/cutlass-fork that referenced this pull request Jul 16, 2024
* fix typos in 02_layout_algebra.md

* fix typos in 03_tensor.md
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.

4 participants