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

Restructure Sphinx Doc #1093

Merged
merged 2 commits into from
Aug 3, 2020
Merged

Conversation

SimeonEhrig
Copy link
Member

@SimeonEhrig SimeonEhrig commented Jul 20, 2020

The current documentation is not well suited for a Sphinx Doc. The refactoring includes two steps:

  1. change the structure
  • introduce new sections: basic, advanced, extra info and development
  • Reorder existing documentation and delete documentation that is no longer relevant
  1. change the content of the sections

TODO to merge

This points have to do, that the PR is ready for review. The points handles inconsistencies in the documentation and general structure problems.

  • Mapping onto specific Hardware: Split CUDA GPUs in General GPUs and CUDA backend section
  • Similar Projects
  • update cheatsheet build docu and script

TODO for future PRs

This points should be done in future PRs.

  • Abstractions section
    • The section has to much text and is outdated.
  • Library Interface
    • Interface usage should be overworked with the slides from the Alpaka CERN workshop
  • Rationale
    • overwork formatting of Implementation Variants
  • Backends
    • improve Accelerator Implementation table

ci_filter: [ ]

- make hierarchy more flat
- new categories: basic, advanced, extra info and development
- just renamed, delete section, cut out section and put together in
 another files and changed heading levels
- update some internal links
@psychocoderHPC
Copy link
Member

psychocoderHPC commented Jul 21, 2020

for this refactoring the last commit should always contain a CI filter with an empty list. see #1077

@SimeonEhrig
Copy link
Member Author

for this refactoring the last commit should always contain a CI filter with an empty list. see #1077

@psychocoderHPC
ci_filter: [] did not work, maybe ci_filter: [ ]
Can you please cancel the CI

@psychocoderHPC
Copy link
Member

psychocoderHPC commented Jul 21, 2020

for this refactoring the last commit should always contain a CI filter with an empty list. see #1077

@psychocoderHPC
ci_filter: [] did not work, maybe ci_filter: [ ]
Can you please cancel the CI

You need to add it to the last commit message of the PR not to the github description.

@SimeonEhrig
Copy link
Member Author

for this refactoring the last commit should always contain a CI filter with an empty list. see #1077

@psychocoderHPC
ci_filter: [] did not work, maybe ci_filter: [ ]
Can you please cancel the CI

You need to add it to the last commit message of the PR not to the github description.

What did I wrong now?

@SimeonEhrig SimeonEhrig marked this pull request as ready for review July 21, 2020 12:09
@SimeonEhrig
Copy link
Member Author

@alpaka-group/alpaka-developers
The restructuring of Sphinx Doc is completed. Most of the work was to change the order of the files, cut and paste, remove some text and change some sentences. An exception is the Introduction page, which was completely overworked. Please check the grammar, the spelling (I know I'm not good at it ;-) ) and if a section has lost its correct meaning due to its new position.

@SimeonEhrig
Copy link
Member Author

Now, alpaka renders the readthedocs side for each PR, see last entry in the CI list.

For this PR, the link is: https://alpaka--1093.org.readthedocs.build/en/1093/

docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
@sbastrakov
Copy link
Member

For some reason I can't comment right there. It seems there is wrong caption formatting just above this with ### One Block Per Thread

@sbastrakov
Copy link
Member

For the sake of the index I suggest changing this title to just Intallation

@sbastrakov
Copy link
Member

Great work @SimeonEhrig ! Some comments are above. I can have another look through if needed later.

@sbastrakov sbastrakov added this to the Version 0.6.0 milestone Jul 21, 2020
@SimeonEhrig
Copy link
Member Author

Thanks @sbastrakov for the great feedback. I think you have found many old errors and some porting errors from the old markdown files in addition to my new errors 😅

@sbastrakov
Copy link
Member

Please find some more comments and questions above. I understand the text is mostly reused from the previous documentation, so did not do all my text suggestions, but just a couple (I can do a follow-up PR for that, as it would be quicker that way).

Copy link
Member

@j-stephan j-stephan left a comment

Choose a reason for hiding this comment

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

First review pass done.

docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/advanced/backends.rst Outdated Show resolved Hide resolved
docs/source/basic/intro.rst Outdated Show resolved Hide resolved
docs/source/basic/intro.rst Outdated Show resolved Hide resolved
docs/source/basic/intro.rst Show resolved Hide resolved
Library Interface
=================

As described in the chapter about the :doc:`Abstraction </basic/abstraction>`, the general design of the library is very similar to *CUDA* and *OpenCL* but extends both by some points, while not requiring any language extensions.
Copy link
Member

Choose a reason for hiding this comment

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

General question: Why are we mentioning OpenCL so often? Except for the experimental SYCL backend we have no relationship to OpenCL.

Copy link
Member

Choose a reason for hiding this comment

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

I think because it was like that at the Master thesis used as a basis for these texts

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it was 2015 😎 . Most parts were copied from Benjamin's master thesis. In my opinion, with overworking the text, we should remove a lot of the OpenCL stuff.

Kokkos provides an abstract interface for portable, performant shared memory-programming. It is a C++ library that offers ``parallel_for``, ``parallel_reduce`` and similar functions for describing the pattern of the parallel tasks. The execution policy determines how the threads are executed. For example, this influences the sizes of blocks of threads or if static or dynamic scheduling should be used. The library abstracts the kernel as a function object that can not have any user defined parameters for its ``operator()``. Arguments have to be stored in members of the function object coupling algorithm and data together. *KOKKOS* provides both, abstractions for parallel execution of code and data management. Multidimensional arrays with a neutral indexing and an architecture dependent layout are available, which can be used, for example, to abstract the underlying hardwares preferred memory access scheme that could be row-major, column-major or even blocked.


`Thrust <https://thrust.github.io/>`_
Copy link
Member

Choose a reason for hiding this comment

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

Is Thrust really a similar library? I feel that Thrust, ArrayFire, ... are much more high-level than alpaka.

Copy link
Member

@sbastrakov sbastrakov Jul 29, 2020

Choose a reason for hiding this comment

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

Well, similar in one way but not on another. I guess this may be left as is, and clarified later.
But I agree they operate on much higher abstraction level, and it's not easy possible to port between those and alpaka

Copy link
Member Author

Choose a reason for hiding this comment

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

It's fine by me, but we can remove it. I thought it would be better to keep two rather than one of more than 10 related works (that was CUDA, OpenCL, ...).

Copy link
Member Author

Choose a reason for hiding this comment

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

I keep it, because it looks better to have two projects and it is not fully unrelated.

@SimeonEhrig
Copy link
Member Author

@sbastrakov @j-stephan
Can you please check if the headings in the introduction are ok? I think this should be the last item before the merge.

The rest of the outstanding tasks should be done in extra PR.

@sbastrakov
Copy link
Member

@SimeonEhrig I do not have objections.

sbastrakov
sbastrakov previously approved these changes Jul 30, 2020
- overworked introduction
- integrate cmake example in introduction site to get a good first
look on alpaka
- split section CUDA GPUs to general GPUs and backend CUDA
- overworked similar projects section
@j-stephan
Copy link
Member

Looks good now. @psychocoderHPC This can be merged.

@sbastrakov sbastrakov merged commit 683ced0 into alpaka-group:develop Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants