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

Initialization.rst: fix superscripts and render code as code #531

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RL-S
Copy link
Contributor

@RL-S RL-S commented Jun 7, 2024

Two superscripts weren't rendered as such, because the leading (escaped) space before ':sup:<num>' was missing. Also, the table of command line arguments was rendered as text, rendering double dashes as em or en dashes (at least in my browser). Finally, removed two extraneous words in the "Initialization by struct" paragraph. Also, why is this .rst, but other parts are .md? I think that leads to a lot of code being simply rendered as italics: anything inside single backticks without "code::" before it.

Two superscripts weren't rendered as such, because the leading (escaped) space before ':sup:`<num>`' was missing.
Also, the table of command line arguments was rendered as text, rendering double dashes as em or en dashes (at least in my browser).
Finally, removed two extraneous words in the "Initialization by struct" paragraph.
Also, why is this .rst, but other parts are .md? I think that leads to a lot of code being simply rendered as italics: anything inside single backticks without "code::" before it.
Copy link
Contributor

@masterleinad masterleinad 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 to me! Thanks!

Copy link
Contributor

@ajpowelsnl ajpowelsnl left a comment

Choose a reason for hiding this comment

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

Just a few details to fix up

@@ -63,26 +63,26 @@ Table 5.1: Command-line options for Kokkos::initialize

* - Argument
- Description
* - --kokkos-help --help
* - :code:`--kokkos-help --help`
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 it's just my_kokkos_executable --kokkos-help

@@ -100,14 +100,17 @@ The values are case insensitive.
5.2 Initialization by environment variable
------------------------------------------

Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we have
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we may use

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you please link to table 5.1?

Copy link
Contributor

Choose a reason for hiding this comment

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

By link, I meant a URL one can click for additional information. Is "< Table clio-pts>" a clickable URL?

Copy link
Contributor Author

@RL-S RL-S Jul 16, 2024

Choose a reason for hiding this comment

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

It's supposed to be one, yes. You click it, and it jumps to Table 5.1. For that, I put an anchor above it, in line 60:
.. Table_cli-opts:
I'm not an expert in reStructured text, and neither is GitHub's preview, so references are a bit iffy. But the syntax
`Table 5.1 <Table_cli-opts>`_
is supposed to display 'Table 5.1' and the reference target is what's within the angle braces (< >). The underscore at the end is necessary, but I don't know, why.

5.3 Initialization by struct
----------------------------

Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the set functions `set_xxx` where `xxx` is the identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions.
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the functions `set_xxx` where `xxx` is identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

A few comments:

  • Please link to Table 5.1
  • In the "If one wants to set a options using the struct," clause, please remove the "a"; the sentence should read, "If one wants to set options using the struct, one can use the functions set_xxx where xxx is identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. "

@ajpowelsnl
Copy link
Contributor

ping @RL-S

response to kokkos#531 (review)

Inserted custom anchor to allow referencing Table 5.1 and used it for all *three* references in the text, removed extraneous 'a' as suggested, and removed `--help` after `--kokkos-help`
@RL-S
Copy link
Contributor Author

RL-S commented Jul 13, 2024

ping @RL-S

Was on holiday until yesterday. See new commit, should resolve it.

@masterleinad masterleinad requested a review from ajpowelsnl July 15, 2024 14:42
Copy link
Contributor

@ajpowelsnl ajpowelsnl left a comment

Choose a reason for hiding this comment

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

It's good once the question around a working link to Table-5.1 is finalized.

@@ -49,40 +49,42 @@ Kokkos chooses the two spaces using the following list:
8. `Kokkos::Threads`
9. `Kokkos::Serial`

The highest execution space in the list that is enabled is Kokkos' default execution space, and the highest enabled host execution space is Kokkos' default host execution space. For example, if `Kokkos::Cuda`, `Kokkos::OpenMP`, and `Kokkos::Serial` are enabled, then `Kokkos::Cuda` is the default execution space and `Kokkos::OpenMP` is the default host execution space.:sup:`1` In cases where the highest enabled backend is a host parallel execution space the `DefaultExecutionSpace` and the `DefaultHostExecutionSpace` will be the same.
The highest execution space in the list that is enabled is Kokkos' default execution space, and the highest enabled host execution space is Kokkos' default host execution space. For example, if `Kokkos::Cuda`, `Kokkos::OpenMP`, and `Kokkos::Serial` are enabled, then `Kokkos::Cuda` is the default execution space and `Kokkos::OpenMP` is the default host execution space\ :sup:`1`. In cases where the highest enabled backend is a host parallel execution space the `DefaultExecutionSpace` and the `DefaultHostExecutionSpace` will be the same.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, is the "highest" execution space formally a Kokkos convention, something approximating operator precedence, or is it a concept we just expect everyone to understand automatically? If it's not been formalized, perhaps a sentence or two defining it might be useful ...?

@@ -100,14 +100,17 @@ The values are case insensitive.
5.2 Initialization by environment variable
------------------------------------------

Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we have
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we may use

Copy link
Contributor

Choose a reason for hiding this comment

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

By link, I meant a URL one can click for additional information. Is "< Table clio-pts>" a clickable URL?

5.3 Initialization by struct
----------------------------

Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the set functions `set_xxx` where `xxx` is the identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions.
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set options using the struct, one can use the functions `set_xxx` where `xxx` is identical to the arguments in `Table 5.1 <Table_cli-opts>`_ where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there API documentation on using the struct for initialization settings? If not, that might be something to do, comparing one method to the other. I have the same question as above about the link to Table 5.1.

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