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

MathiasMagnus SAXPY tutorial #3487

Merged
merged 1 commit into from
Jun 25, 2024
Merged

MathiasMagnus SAXPY tutorial #3487

merged 1 commit into from
Jun 25, 2024

Conversation

neon60
Copy link
Contributor

@neon60 neon60 commented May 17, 2024

The SAXPY tutorial.

@neon60 neon60 marked this pull request as draft May 17, 2024 14:34
@neon60 neon60 changed the base branch from develop to docs/develop May 17, 2024 14:39
@neon60 neon60 marked this pull request as ready for review May 20, 2024 07:51
@neon60 neon60 changed the title WIP - MathiasMagnus SAXPY and Reduction tutorial MathiasMagnus SAXPY and Reduction tutorial May 20, 2024
Copy link

@randyh62 randyh62 left a comment

Choose a reason for hiding this comment

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

provided comments

docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
docs/tutorials/reduction.rst Outdated Show resolved Hide resolved
@neon60
Copy link
Contributor Author

neon60 commented May 23, 2024

IMAGE OF FINAL ALGO is missing

@neon60
Copy link
Contributor Author

neon60 commented May 27, 2024

IMAGE OF FINAL ALGO is missing

Fixed.

@neon60 neon60 force-pushed the saxpy_reduction_tutorial branch 2 times, most recently from d919307 to b3fbc85 Compare May 28, 2024 12:04
@peterjunpark
Copy link

Added language suggestions from @lpaoletti. Review the diff if needed 2b4a8be

@neon60
Copy link
Contributor Author

neon60 commented Jun 5, 2024

@lpaoletti Can I get an approval?

@neon60 neon60 changed the title MathiasMagnus SAXPY and Reduction tutorial MathiasMagnus SAXPY tutorial Jun 5, 2024
@neon60
Copy link
Contributor Author

neon60 commented Jun 5, 2024

The reduction tutorial PR: #3513

@peterjunpark
Copy link

peterjunpark commented Jun 5, 2024

Should we remove the tab sync?

I don't like how the page jumps around when you select a tab due to the length of content changing all over the place at the same time.

@neon60
Copy link
Contributor Author

neon60 commented Jun 5, 2024

Should we remove the tab sync?

I don't like how the page jumps around when you select a tab due to content lengths changing everywhere at the same time.

Probably we should. I am thinking what we can do with it.

@randyh62
Copy link

randyh62 commented Jun 5, 2024

Should we remove the tab sync?
I don't like how the page jumps around when you select a tab due to content lengths changing everywhere at the same time.

Probably we should. I am thinking what we can do with it.

I think we should keep it. I think a user is going to pick their flow in the first tab, and the page will update the content to display their flow. I don't think people will be flipping back and forth between flows except to see what differences there might be.

docs/tutorial/saxpy.rst Outdated Show resolved Hide resolved
docs/tutorial/saxpy.rst Show resolved Hide resolved
============================

First, let's do the "Hello, World!" of GPGPU: SAXPY. *SAXPY* is a mathematical
acronym; a vector equation :math:`a\cdot x+y=z` where :math:`a\in\mathbb{R}` is
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you have mentioned SAXPY's full form below. Can we move it up here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.


``HIP_CHECK`` is a custom macro borrowed from the examples utilities which
checks the error code returned by API functions for errors and reports them to
the console. It's not essential to the API.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not essential to the API but it is a good practice to check the error codes of the HIP APIs incase you pass on incorrect values to the API or the API might be out of resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

This function is launched from the host using a language extension often called
the triple chevron syntax. Inside the angle brackets, provide the following.

- The number of blocks to launch (our grid size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any documentation describing grid and block sizes? If yes can we point the user to it from here. I think these are non trivial concepts and user who just wants to exec stuff on their GPU can go it it if they want to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

- A check is made to avoid overindexing the input.
- The useful part of the computation is carried out.

Retrieval of the result from the device is done much like its dispatching:
Copy link
Contributor

Choose a reason for hiding this comment

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

dispatching can mean different thing in context of HIP, can we change the wording here. Also the first memcpy should also be explained and here we can mention in simple terms that we are copying result back to host.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed dispatching.

docs/tutorial/saxpy.rst Show resolved Hide resolved
.. tab-item:: Linux and AMD
:sync: linux-amd

The utilities included with ROCm help significantly to inspect binary
Copy link
Contributor

Choose a reason for hiding this comment

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

I think binary inspection deserves its own doc, rather then putting it alongside saxpy explanation. In my head person looking for saxpy documentation will be least bothered with code object extraction and the person wanting to play with code objects will be least bothered with saxpy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can put this feedback to a separate issue. Currently, we do not have binary inspection tutorial and at least we have some here now.

If we finally have the binary inspection page, we can rethink this suggestion. Maybe I can put this into a dropdown:
https://sphinx-design.readthedocs.io/en/latest/dropdowns.html

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather have its own page, playing with code objects might require a bit more detail and populating here will overcomplicate this tutorial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I am creating a separate issue to remove the code inspection from here to a new code inspection tutorial.

The Boy Scout Rule: Leave the campground cleaner than you found it. → I am merging this in. One tutorial better than zero, and we will improve this later.

Copy link
Contributor

@cjatin cjatin left a comment

Choose a reason for hiding this comment

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

Looks good

.. tab-item:: Linux and AMD
:sync: linux-amd

The utilities included with ROCm help significantly to inspect binary
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather have its own page, playing with code objects might require a bit more detail and populating here will overcomplicate this tutorial.

docs/tutorial/saxpy.rst Show resolved Hide resolved
@neon60
Copy link
Contributor Author

neon60 commented Jun 25, 2024

Looks good

Thanks for the review.

- Move md files to rst
- Impl editorial suggestions from @lpaoletti
- External PR feedbacks from HIP team
@neon60 neon60 merged commit 1bf5ffa into docs/develop Jun 25, 2024
4 checks passed
@neon60 neon60 deleted the saxpy_reduction_tutorial branch June 29, 2024 10:44
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.

5 participants