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.

Sign up for GitHub

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

Jump to bottom

Adds initial files for sphinx/rst and readthedocs. #990

Merged
merged 1 commit into from
May 24, 2020
Merged

Adds initial files for sphinx/rst and readthedocs. #990

merged 1 commit into from
May 24, 2020

Conversation

tdd11235813
Copy link
Contributor

@tdd11235813 tdd11235813 commented Apr 27, 2020 •
edited
Loading

Converts documentation to sphinx/rst for readthedocs

Changes

  • migrates from markdown to rst formatted documentation
    • adds sphinx+readthedocs documentation
    • fixes links in documentation files
    • adds how to enable HTML doxygen target
    • adds API reference link to toctree
    • sphinx: tested html, including readthedocs, and latexpdf output
  • removes markdown/
  • doc/ -> docs/
  • moves images folder to docs/source/ (to have absolute paths in rst)
  • own logo folder for alpaka logos
  • updates README.md link to logo
    • adds rtd and doxygen status badges to README.md
  • Doxygen: XML output only, run script uses sed to enable HTML
    • sphinx/breathe only requires XML
  • sphinx Makefile with target for link-checking

Versions: sphinx==3.0.3 & breathe==4.16.0

  • Note that Breathe 4.13.0 drops support for Sphinx<2.0
  • Note that Sphinx 2.0 requires Python 3.5+
  • Note that breathe 4.15.0 would require Sphinx 3.0+

See result here.

This PR refers to #943.

Note that after merging this PR, I configure readthedocs to upstream. An updated branch will trigger readthedocs/sphinx to recompile the online doc.

Sidenote: the github alpaka workflow link in README.md should refer to new alpaka-group page, I guess

Outdated:
Sections included:
- Introduction
- Abstraction
- Implementation
- Code Formatting
Sections not included yet:
- all the nested subsections (Thread, Block, ..., Library Interface, Mapping)
Changes
- removes doc/markdown/ and moves pics to doc/images/.
- doc/ -> docs/
- own logo folder for alpaka logos.
- updates README.md link to logo
- adds readthedocs and doxygen status badges to README.md

@SimeonEhrig
Copy link
Member

Can you please add a section under contribution which describes how to build the sphinx doc? It is not too hard, but for somebody who has no experience with Python and Sphinx it can helps a lot of.

@BenjaminW3
Copy link
Member

I can continue porting

I would really like to see this!

BenjaminW3
BenjaminW3 previously approved these changes Apr 29, 2020
View reviewed changes
@BenjaminW3
Copy link
Member

How does readthedocs work?
Did you have to reserve the alpaka name? With which account did you do this? When is this documentation updated?

@tdd11235813
Copy link
Contributor Author

How does readthedocs work?
Did you have to reserve the alpaka name? With which account did you do this? When is this documentation updated?

I'll add readthedocs/sphinx as section to alpaka.

The organization feature of readthedocs is only in the business version available. So the alpaka readthedocs is tied to three maintainers (@psychocoderHPC , @SimeonEhrig and me).
Please contact @psychocoderHPC if more maintainers need to be added.

He also could remove me as maintainer for a moment, so we can try what happens, when I am out.
I created the readthedocs URL for alpaka with my account, but it does not seem to be bound on that.
I'll update it as soon as possible ;) I guess you need the missing sections, too, in order to have a complete doc, although you might want to refactor it later on. The code style guide should be reviewed, too.

@BenjaminW3
Copy link
Member

The code style guide will mostly get removed once I finished the clang-format integration. So please do not do too much work there.

@psychocoderHPC
Copy link
Member

@BenjaminW3
I can add you as maintainer if I forger this please remember me I am currently afk.
@tdd11235813 Alpaka is open source feel free to stay readthedocs maintainer.

@tdd11235813
Copy link
Contributor Author

ok, fine with me :)

Info: I am about to integrate the remaining sections and I am playing with breathe to include doxygen information into sphinx. I followed mostly picongpu's workflow, i.e. doxygen Doxyfile has only XML target enabled (for breathe) and a sed line re-enables it in our run_doxygen.sh.
2x Doxyfiles (HTML, XML) would be too redundant, and doxygen does not offer CLI for target selection, and always creating both targets also does not seem efficient.

@tdd11235813
Copy link
Contributor Author

update is online, also check my branch to see the docs/ structure and the README.md update.

  • make latexpdf produces an error, can you reproduce it? Seems weird, not sure how to solve it at the moment

Syntax Error: Couldn't find trailer dictionary
Syntax Error: Couldn't read xref table

!pdfTeX error: pdflatex (file ./alpaka.pdf): xpdf: reading PDF image failed
==> Fatal error occurred, no output PDF file produced!

  • make html shows warnings about doxygen generated things on a function, I actually want to ignore this at this stage
/alpaka/docs/source/usage/implementation/mapping/CUDA.rst:60: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 98]
    template<typename T, std::size_t TuniqueId, typename TBlockSharedMemSt>ALPAKA_NO_HOST_ACC_WARNING ALPAKA_FN_ACC auto alpaka::block::shared::st::allocVar(TBlockSharedMemSt const & blockSharedMemSt)
    --------------------------------------------------------------------------------------------------^

BenjaminW3
BenjaminW3 previously approved these changes May 6, 2020
View reviewed changes
SimeonEhrig
SimeonEhrig reviewed May 6, 2020
View reviewed changes
Copy link
Member

@SimeonEhrig SimeonEhrig left a comment

Choose a reason for hiding this comment

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

update is online, also check my branch to see the docs/ structure and the README.md update.

* `make latexpdf` produces an **error**, can you reproduce it? Seems weird, not sure how to solve it at the moment

Syntax Error: Couldn't find trailer dictionary
Syntax Error: Couldn't read xref table
!pdfTeX error: pdflatex (file ./alpaka.pdf): xpdf: reading PDF image failed
==> Fatal error occurred, no output PDF file produced!

* `make html` shows warnings about doxygen generated things on a function, I actually want to ignore this at this stage

/alpaka/docs/source/usage/implementation/mapping/CUDA.rst:60: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 98]
    template<typename T, std::size_t TuniqueId, typename TBlockSharedMemSt>ALPAKA_NO_HOST_ACC_WARNING ALPAKA_FN_ACC auto alpaka::block::shared::st::allocVar(TBlockSharedMemSt const & blockSharedMemSt)
    --------------------------------------------------------------------------------------------------^

I can reproduce the pdf error and html warning.

docs/source/dev/sphinx.rst Outdated Show resolved Hide resolved
@SimeonEhrig
Copy link
Member

It may be useful to add a link to the doxygen documentation on the left side of the section overview (as with a normal section). The first time, I missed the link on the main page and did a little search.

@psychocoderHPC
Copy link
Member

I have not followed the review but do we need to add the token to readthedocs or any other platform before we merge this PR?

@psychocoderHPC psychocoderHPC added this to the Version 0.5.0 milestone May 6, 2020
@tdd11235813 tdd11235813 force-pushed the pr-readthedocs branch 2 times, most recently from 97d9392 to fc1de2f Compare May 12, 2020 12:33
@tdd11235813
Copy link
Contributor Author

tdd11235813 commented May 12, 2020 •
edited
Loading

  • latexpdf now works
    • sphinx structures files like logo in a flat folder, so name conflict happened with the output PDF file, which had the same name alpaka.pdf like the logo (actually pdflatex included its own unfinished output file as logo^^)
    • alpaka.tex -> alpaka-doc.tex -> alpaka-doc.pdf to avoid name conflict
  • Adds documentation (sphinx.rst) how to enable HTML doxygen target
  • Adds API reference link to toctree

@psychocoderHPC asked for token, here I need instructions for that, what you need exactly:

I have not followed the review but do we need to add the token to readthedocs or any other platform before we merge this PR?

Edit: rebased with develop. Going to squash commits after review process has been finished.

@SimeonEhrig
Copy link
Member

Looks really good. Great Job!
I can confirm, that the local build of Doxygen, html and latexpdf works.

Last question: Is automatic build via CI already configured?

@tdd11235813
Copy link
Contributor Author

Last question: Is automatic build via CI already configured?

Note that after merging this PR, I configure readthedocs to upstream. An updated branch will trigger readthedocs/sphinx to recompile the online doc.

Squashed commits and updated starter post with changes to be committed to upstream.

SimeonEhrig
SimeonEhrig previously approved these changes May 18, 2020
View reviewed changes
@BenjaminW3
Copy link
Member

You still have to fix a conflict in the readme before this can be merged.

Converts documentation to sphinx/rst for readthedocs
e442862 
Changes
- migrates from markdown to rst formatted documentation
  - adds sphinx+readthedocs documentation
  - fixes links in documentation files
  - adds how to enable HTML doxygen target
  - adds API reference link to toctree
  - sphinx: tested html, including readthedocs, and latexpdf output
- removes markdown/
- doc/ -> docs/
- moves images folder to docs/source/ (to have absolute paths in rst)
- own logo folder for alpaka logos
- updates README.md link to logo
  - adds rtd and doxygen status badges to README.md

- Note that Breathe 4.13.0 drops support for Sphinx<2.0
- Note that Sphinx 2.0 requires Python 3.5+
- Note that breathe 4.15.0 would require Sphinx 3.0+

- Doxygen: XML output only, run script uses sed to enable HTML
  - sphinx/breathe only requires XML
- sphinx Makefile with target for link-checking
@BenjaminW3 BenjaminW3 merged commit 04dffc8 into alpaka-group:develop May 24, 2020
@tdd11235813
Copy link
Contributor Author

alpaka readthedocs configured to upstream. Currently uses only 'latest' build, inactivated 'stable'. You can configure builds on readthedocs in "Versions".

@ax3l
Copy link
Member

ax3l commented May 29, 2020 •
edited
Loading

Wuiii, so cool!

@tdd11235813 @BenjaminW3 @SimeonEhrig We recently found a trick with @MaxThevenet how to even include the Doxygen API HTML in versioned Sphinx HTML builds for e.g. RTD. Here is how we do it: openPMD/openPMD-api#697

I even went one step further and created a doxygen index, which you can use with Xeus-Cling for in-jupyter ?-help on APIs: https://openpmd-api.readthedocs.io/en/latest/details/doxygen.html
conda-forge/openpmd-api-feedstock@1d19a5e (Current limitation: jupyter-xeus/xeus-cling#318)

@SimeonEhrig SimeonEhrig mentioned this pull request Jun 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminW3 BenjaminW3 BenjaminW3 left review comments

@SimeonEhrig SimeonEhrig SimeonEhrig left review comments

Assignees
No one assigned
Labels
Type:Documentation
Projects
None yet
Milestone
Version 0.5.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tdd11235813 @SimeonEhrig @BenjaminW3 @psychocoderHPC @ax3l