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

Generate unified Python/C++ docs #1324

Merged
merged 22 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conda/environments/all_cuda-118_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ channels:
- rapidsai
- conda-forge
dependencies:
- breathe
- c-compiler
- clang-tools==16.0.6
- clang==16.0.6
Expand Down
1 change: 1 addition & 0 deletions conda/environments/all_cuda-120_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ channels:
- rapidsai
- conda-forge
dependencies:
- breathe
- c-compiler
- clang-tools==16.0.6
- clang==16.0.6
Expand Down
1 change: 1 addition & 0 deletions dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ dependencies:
common:
- output_types: conda
packages:
- breathe
vyasr marked this conversation as resolved.
Show resolved Hide resolved
- *doxygen
- graphviz
- ipython
Expand Down
2 changes: 1 addition & 1 deletion include/rmm/device_uvector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace rmm {
* `thrust::uninitialized_fill`.
*
* Example:
* @code{c++}
* @code{.cpp}
* rmm::mr::device_memory_resource * mr = new my_custom_resource();
* rmm::cuda_stream_view s{};
*
Expand Down
4 changes: 3 additions & 1 deletion include/rmm/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ struct bytes {

/**
* @brief Returns the global RMM logger
* @addtogroup logging
*
* @ingroup logging
*
* This is a spdlog logger. The easiest way to log messages is to use the `RMM_LOG_*` macros.
*
* @return spdlog::logger& The logger.
Expand Down
2 changes: 1 addition & 1 deletion include/rmm/mr/device/device_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ namespace rmm::mr {
* pool_memory_resource objects for each device and sets them as the per-device resource for that
* device.
*
* @code{c++}
* @code{.cpp}
* std::vector<unique_ptr<pool_memory_resource>> per_device_pools;
* for(int i = 0; i < N; ++i) {
* cudaSetDevice(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ using failure_callback_t = std::function<bool(std::size_t, void*)>;
* When implementing a callback function for allocation retry, care must be taken to avoid an
* infinite loop. The following example makes sure to only retry the allocation once:
*
* @code{c++}
* @code{.cpp}
Copy link
Member

Choose a reason for hiding this comment

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

Does it work to just leave off the {.cpp}? By default, Doxygen uses the language of the file the @code is found in. Will that work for the unified case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately doxygen-generated XML does not correctly identify hpp files as C++ files. It does work for cpp files, but since rmm's files are all headers I needed to specify this. I do wonder if updating to a newer version of doxygen would improve the situation, and I may test that. I think #1317 needs merging before an y PR to update doxygen, so if the test is promising I'll sequence that work appropriately.

Copy link
Member

Choose a reason for hiding this comment

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

Try modifying the Doxyfile EXTENSION_MAPPING setting?

EXTENSION_MAPPING      = cu=C++ \
                         cuh=C++ \
                         hpp=C++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That doesn't seem to work, but I'm not sure why. Oddly, specifying @code{.hpp} works (whether or not hpp is added to the EXTENSION_MAPPING), but the default inference of just @code doesn't.

* using failure_callback_adaptor =
* rmm::mr::failure_callback_resource_adaptor<rmm::mr::device_memory_resource>;
*
Expand Down
2 changes: 1 addition & 1 deletion include/rmm/mr/device/per_device_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
* pool_memory_resource objects for each device and sets them as the per-device resource for that
* device.
*
* @code{c++}
* @code{.cpp}
* std::vector<unique_ptr<pool_memory_resource>> per_device_pools;
* for(int i = 0; i < N; ++i) {
* cudaSetDevice(i);
Expand Down
2 changes: 1 addition & 1 deletion include/rmm/mr/device/polymorphic_allocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ bool operator!=(polymorphic_allocator<T> const& lhs, polymorphic_allocator<U> co
*`deallocate` functions.
*
* Example:
*\code{c++}
*\code{.cpp}
* my_stream_ordered_allocator<int> a{...};
* cuda_stream_view s = // create stream;
*
Expand Down
2 changes: 1 addition & 1 deletion python/docs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# You can set these variables from the command line, and also
# from the environment for the first two.
SPHINXOPTS = -n -v -W --keep-going
SPHINXOPTS = -n -v
vyasr marked this conversation as resolved.
Show resolved Hide resolved
SPHINXBUILD ?= sphinx-build
SOURCEDIR = .
BUILDDIR = _build
Expand Down
11 changes: 11 additions & 0 deletions python/docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,12 @@
"IPython.sphinxext.ipython_directive",
"nbsphinx",
"recommonmark",
"breathe",
]

# Breathe Configuration
breathe_projects = {"librmm": "../../doxygen/xml"}
breathe_default_project = "librmm"

copybutton_prompt_text = ">>> "

Expand Down Expand Up @@ -197,9 +201,16 @@
]


# def on_missing_reference(app, env, node, contnode):
# if node["refdomain"] == "cpp":
# return contnode
# return None


def setup(app):
app.add_js_file("copybutton_pydocs.js")
app.add_css_file("https://docs.rapids.ai/assets/css/custom.css")
app.add_js_file(
"https://docs.rapids.ai/assets/js/custom.js", loading_method="defer"
)
# app.connect("missing-reference", on_missing_reference)
8 changes: 8 additions & 0 deletions python/docs/cpp.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Welcome to the rmm C++ documentation!
========================================
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

This shows up in the navigation bar (they call these "breadcrumbs").

image

I would prefer to do something like:

Suggested change
Welcome to the rmm C++ documentation!
========================================
rmm C++
=======

Copy link
Member

Choose a reason for hiding this comment

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

I agree -- never liked the "Welcome to" prefix on practically every Python package's docs.


.. toctree::
:maxdepth: 2
:caption: Contents:

cpp_api.rst
8 changes: 8 additions & 0 deletions python/docs/cpp_api.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
API Reference
=============

.. toctree::
:maxdepth: 2
:caption: Contents:

librmm_docs/index
2 changes: 1 addition & 1 deletion python/docs/basics.md → python/docs/guide.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# RMM - the RAPIDS Memory Manager
# User Guide

Achieving optimal performance in GPU-centric workflows frequently requires
customizing how GPU ("device") memory is allocated.
Expand Down
4 changes: 2 additions & 2 deletions python/docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ Welcome to rmm's documentation!
:maxdepth: 2
:caption: Contents:

basics.md
api.rst
Python <python.rst>
C++ <cpp.rst>


Indices and tables
Expand Down
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/cuda_device_management.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CUDA Device Management
======================

.. doxygengroup:: cuda_device_management
:members:
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/cuda_streams.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CUDA Streams
============

.. doxygengroup:: cuda_streams
:members:
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/data_containers.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Data Containers
===============

.. doxygengroup:: data_containers
:members:
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/errors.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Errors
======

.. doxygengroup:: errors
:members:
29 changes: 29 additions & 0 deletions python/docs/librmm_docs/index.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
.. rmm documentation master file, created by
sphinx-quickstart on Thu Nov 19 13:16:00 2020.
You can adapt this file completely to your liking, but it should at least
contain the root `toctree` directive.

librmm Documentation
====================
Comment on lines +6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

This heading makes the breadcrumbs deeply nested. The navigation is awkward as a result.

image

Can we eliminate this level of the hierarchy entirely, so that it's something more like Home / rmm C++ / API Reference / Memory Resources? See if you can fuse this page's toctree into python/docs/cpp.rst instead.


.. toctree::
:maxdepth: 2
:caption: Contents:

memory_resources
data_containers
thrust_integrations
cuda_device_management
cuda_streams
errors
logging


.. doxygennamespace:: rmm
:desc-only:

Indices and tables
==================

* :ref:`genindex`
* :ref:`search`
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/logging.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Logging
=======

.. doxygengroup:: logging
:members:
17 changes: 17 additions & 0 deletions python/docs/librmm_docs/memory_resources.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Memory Resources
================

.. doxygennamespace:: rmm::mr
:desc-only:

.. doxygengroup:: memory_resources
:desc-only:

.. doxygengroup:: device_memory_resources
:members:

.. doxygengroup:: host_memory_resources
:members:

.. doxygengroup:: device_resource_adaptors
:members:
5 changes: 5 additions & 0 deletions python/docs/librmm_docs/thrust_integrations.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Thrust Integration
==================

.. doxygengroup:: thrust_integrations
:members:
9 changes: 9 additions & 0 deletions python/docs/python.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Welcome to the rmm Python documentation!
========================================
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, this makes for an awkward breadcrumb.

image

Suggested change
Welcome to the rmm Python documentation!
========================================
rmm Python
==========


.. toctree::
:maxdepth: 2
:caption: Contents:

guide.md
python_api.rst
File renamed without changes.