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

update documentation #12

Open
wants to merge 9 commits into
base: amd-staging
Choose a base branch
from
Open

update documentation #12

wants to merge 9 commits into from

Conversation

SwRaw
Copy link
Contributor

@SwRaw SwRaw commented Nov 27, 2024

No description provided.

@SwRaw SwRaw requested a review from a team as a code owner November 27, 2024 14:45
samjwu and others added 2 commits November 28, 2024 15:00
Change-Id: Ie53617712a3762cee1f0728e6f7fa2b654d1c425
Change-Id: Idcf882fcc73c5d80f49db155d554e69d24c2b942
Comment on lines 20 to 49
- A C++ 17 compiler such as GCC 7 or Clang 5.

- The AMD ROCm software stack. See the `ROCm installation instructions <https://rocm.docs.amd.com/projects/install-on-linux/en/latest/index.html>`_.
- The AMD ROCm software stack. See the `ROCm installation instructions <rocm-install-on-linux:index>`.

- Install the required packages according to the OS:

- Packages as per the OS.
.. tab-set::

- For Ubuntu 18.04 and Ubuntu 20.04:
.. tab-item:: Ubuntu 18.04 and Ubuntu 20.04
:sync: ubuntu

.. code:: shell
.. code-block:: shell

apt install gcc g++ make cmake libelf-dev libdw-dev
apt install gcc g++ make cmake libelf-dev libdw-dev

- For CentOS 8.1 and RHEL 8.1:
.. tab-item:: CentOS 8.1 and RHEL 8.1
:sync: rhel

.. code:: shell
.. code-block:: shell

yum install gcc gcc-c++ make cmake elfutils-libelf-devel elfutils-devel
yum install gcc gcc-c++ make cmake elfutils-libelf-devel elfutils-devel

- For SLES 15 Service Pack 1:
.. tab-item:: SLES 15 Service Pack 1
:sync: sles

.. code:: shell
.. code-block:: shell

zypper install gcc gcc-c++ make cmake libelf-devel libdw-devel
zypper install gcc gcc-c++ make cmake libelf-devel libdw-devel

- Python 3.6 or later to run the tests.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lancesix Could you please check if these are current? These look outdated to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lancesix Are the packages and OS versions up-to-date?

README.md Outdated
@@ -1,6 +1,9 @@
AMD ROCm Debug Agent Library (ROCdebug-agent)
=============================================

> [!NOTE]
> The published documentation is available at [ROCR Debug Agent](https://rocm.docs.amd.com/projects/rocr_debug_agent/en/latest/index.html) in an organized, easy-to-read format, with search and a table of contents. The documentation source files reside in the `rocr_debug_agent/docs` folder of this repository. As with all ROCm projects, the documentation is open source. For more information on contributing to the documentation, see [Contribute to ROCm documentation](https://rocm.docs.amd.com/en/latest/contribute/contributing.html).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file uses a line length of 80 chars. Can that paragraph be re-flowed match the rest oft he document?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just broke it down in small sentences per line. I hope this is what you implied?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The text should not be impacted, and the line break is not necessarily before a new phrase either.

We just go to the next line if the next word would go over the 80 char limit. Most text editors have a way to do that automatically (:set textwidth=80 for vim, https://marketplace.visualstudio.com/items?itemName=stkb.rewrap plugin for visual studio code, M-x auto-fill-mode for emacs)


.. _installation:

==================================
Installation
ROCR debug agent installation
==================================
Copy link
Collaborator

Choose a reason for hiding this comment

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

The length of the ===[…]=== lines seems off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lancesix I didn't get this one. The length of ==== line seems uniform to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't it be:

Suggested change
==================================
=============================
ROCR debug agent installation
=============================

i.e. 4 less "=", so it matches the width of the title?

note that this is not a hard requirement, this is just pure cosmetics. The rendered output should still be the same regardless


- For Ubuntu 18.04 and Ubuntu 20.04:
.. tab-item:: Ubuntu 18.04 and Ubuntu 20.04
Copy link
Collaborator

Choose a reason for hiding this comment

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

Those could be upgraded. I think 20.04 is "officially" unsupported (or close to be), 18.04 is probably not supported anymore. Currently supported LTS releases are 22.04 and 24.04.

That being said, the dependencies we have are quite stable and should be the same across all available ubuntu versions.

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 will update this. Hope this doesn't impact the install commands.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Install commands and package names have not changed. The only thing is that the distro version might end-up out of sync with https://rocm.docs.amd.com/projects/install-on-linux/en/latest/reference/system-requirements.html.

Maybe it would be easier to just drop the distro versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the newer versions. Please check.


- For CentOS 8.1 and RHEL 8.1:
.. tab-item:: CentOS 8.1 and RHEL 8.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably can add RHEL 9 in the mix here.

So I think this is valid for RHEL 8 and RHEL 9. CentOS 8 (I think) still around as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.


- For SLES 15 Service Pack 1:
.. tab-item:: SLES 15 Service Pack 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just say SLES 15? This is not limited to SP1. I think current version is SP6, but that does not change much for us, so not specifying it is probably easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@SwRaw
Copy link
Contributor Author

SwRaw commented Dec 4, 2024

@lancesix Please review the PR and approve if it looks ok.

docs/index.rst Outdated

.. _index:

===============================
ROCR Debug Agent documentation
===============================

The ROCR Debug Agent (ROCdebug-agent) is a library that can be loaded by the ROCm software runtime `(ROCR) <https://rocm.docs.amd.com/projects/ROCR-Runtime/en/latest/>`_ to provide the following functionalities:
The ROCR Debug Agent (ROCdebug-agent) is a library that can be loaded by the ROCm software runtime `(ROCR) <rocr-runtime:index>` to provide the following functionalities:
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I build the doc locally, that "rocr-runtime:index text appears in the output. Is there another way to build the doc to have cross-docs links?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


- For CentOS 8.1 and RHEL 8.1:
.. tab-item:: CentOS 8.1 and RHEL 8/9
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://rocm.docs.amd.com/projects/install-on-linux/en/latest/reference/system-requirements.html does not list CentOS in the supported systems. Should we drop it? I guess, if we keep it, it should be "CentOS 8", not 8.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


- Python 3.6 or later to run the tests.

- ROCdbgapi library. This can be installed using the ROCdbgapi package as part of the ROCm release. See the instructions to install `ROCdbgapi library <https://rocm.docs.amd.com/projects/ROCdbgapi/en/latest/>`_.
- `ROCdbgapi library <rocdbgapi:index>`. This can be installed using the ROCdbgapi package as part of the ROCm release. See the instructions to install `ROCdbgapi library <https://rocm.docs.amd.com/projects/ROCdbgapi/en/latest/>`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same question, when I render locally, I have "rocdbgapi:index" verbatim and no link. Is that expected?

Also, shouldn't be instruction to install be pointing to https://rocm.docs.amd.com/projects/ROCdbgapi/en/latest/install/build.html ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this.

@SwRaw
Copy link
Contributor Author

SwRaw commented Dec 9, 2024

@lpaoletti Please review

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