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 all 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
11 changes: 8 additions & 3 deletions include/rmm/cuda_stream_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ static const cuda_stream_view cuda_stream_per_thread{
cudaStreamPerThread // NOLINT(cppcoreguidelines-pro-type-cstyle-cast)
};

// Need to avoid putting is_per_thread_default and is_default into the group twice.
/** @} */ // end of group

[[nodiscard]] inline bool cuda_stream_view::is_per_thread_default() const noexcept
{
#ifdef CUDA_API_PER_THREAD_DEFAULT_STREAM
Expand All @@ -134,9 +137,6 @@ static const cuda_stream_view cuda_stream_per_thread{
#endif
}

/**
* @brief Return true if the wrapped stream is explicitly the CUDA legacy default stream.
*/
[[nodiscard]] inline bool cuda_stream_view::is_default() const noexcept
{
#ifdef CUDA_API_PER_THREAD_DEFAULT_STREAM
Expand All @@ -146,6 +146,11 @@ static const cuda_stream_view cuda_stream_per_thread{
#endif
}

/**
* @addtogroup cuda_streams
* @{
*/

/**
* @brief Equality comparison operator for streams
*
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/cuda_async_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class cuda_async_memory_resource final : public device_memory_resource {
* If the pool size grows beyond the release threshold, unused memory held by the pool will be
* released at the next synchronization event.
*
* @throws rmm::runtime_error if the CUDA version does not support `cudaMallocAsync`
* @throws rmm::logic_error if the CUDA version does not support `cudaMallocAsync`
*
* @param initial_pool_size Optional initial size in bytes of the pool. If no value is provided,
* initial pool size is half of the available GPU memory.
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
7 changes: 6 additions & 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 All @@ -72,6 +72,10 @@
*/

namespace rmm::mr {
/**
* @addtogroup memory_resources
* @{
*/

namespace detail {

Expand Down Expand Up @@ -233,4 +237,5 @@ inline device_memory_resource* set_current_device_resource(device_memory_resourc
{
return set_per_device_resource(rmm::get_current_cuda_device(), new_mr);
}
/** @} */ // end of group
} // namespace rmm::mr
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
68 changes: 68 additions & 0 deletions python/docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# documentation root, use os.path.abspath to make it absolute, like shown here.
#
import os
import re

# -- Project information -----------------------------------------------------

Expand Down Expand Up @@ -46,8 +47,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 +202,72 @@
]


def on_missing_reference(app, env, node, contnode):
if (refid := node.get("refid")) is not None and "hpp" in refid:
# We don't want to link to C++ header files directly from the
# Sphinx docs, those are pages that doxygen automatically
# generates. Adding those would clutter the Sphinx output.
return contnode

names_to_skip = [
# External names
"cudaStream_t",
"cudaStreamLegacy",
"cudaStreamPerThread",
"thrust",
"spdlog",
# Unknown types
"int64_t",
"int8_t",
# Internal objects
"detail",
"RMM_EXEC_CHECK_DISABLE",
"default_alignment_threshold",
"get_default_filename",
# Template types
"Base",
]
if (
node["refdomain"] == "cpp"
and (reftarget := node.get("reftarget")) is not None
):
if any(toskip in reftarget for toskip in names_to_skip):
return contnode

# Strip template parameters and just use the base type.
if match := re.search("(.*)<.*>", reftarget):
reftarget = match.group(1)

# Try to find the target prefixed with e.g. namespaces in case that's
# all that's missing. Include the empty prefix in case we're searching
# for a stripped template.
extra_prefixes = ["rmm::", "rmm::mr::", "mr::", ""]
for (name, dispname, type, docname, anchor, priority) in env.domains[
"cpp"
].get_objects():

for prefix in extra_prefixes:
if (
name == f"{prefix}{reftarget}"
or f"{prefix}{name}" == reftarget
):
return env.domains["cpp"].resolve_xref(
env,
docname,
app.builder,
node["reftype"],
name,
node,
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
:members:

.. 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.