From e44cdff34b9e159c9f40b071b357a88a657437c2 Mon Sep 17 00:00:00 2001 From: Shaked Regev Date: Wed, 4 Jun 2025 18:35:08 -0400 Subject: [PATCH 1/5] restructured documentation files --- docs/index.rst | 8 +- .../developer_guide/coding_guidelines.rst | 17 +- .../sphinx/developer_guide/git_guidelines.rst | 14 +- docs/sphinx/developer_guide/index.rst | 156 +++++++----------- docs/sphinx/user_guide/index.rst | 34 ++++ 5 files changed, 124 insertions(+), 105 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 08443fc13..4ae0de3dc 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -30,7 +30,7 @@ Authors and acknowledgment Primary authors of this project are: -* Kasia Świrydowicz kasia.swirydowicz@pnnl.gov (PNNL) +* Kasia Świrydowicz Kasia.Swirydowicz@amd.com (AMD) * Slaven Peles peless@ornl.gov (ORNL) ReSolve project would not be possible without significant contributions @@ -83,7 +83,9 @@ Copyright © 2023, UT-Battelle, LLC, and Battelle Memorial Institute. :caption: Developer Resources doxygen/index + sphinx/developer_guide/coding_guidelines + sphinx/developer_guide/git_guidelines sphinx/developer_guide/index sphinx/developer_guide/profiling - sphinx/developer_guide/git_guidelines - sphinx/developer_guide/coding_guidelines + + diff --git a/docs/sphinx/developer_guide/coding_guidelines.rst b/docs/sphinx/developer_guide/coding_guidelines.rst index c2742bfba..ada82e7f1 100644 --- a/docs/sphinx/developer_guide/coding_guidelines.rst +++ b/docs/sphinx/developer_guide/coding_guidelines.rst @@ -176,7 +176,22 @@ parenthesis as shown here: // some code } -Do not use one-line ``if``\ s and ``for``\ s. Always use braces. +Do not use one-line ``for``\ s. Always use braces. +One-line ``if``\ s are acceptable (though not mandatory) if the command directly follows from the conditional. +I.e. the command can only be executed if the conditional is true. +An ``if`` statement where the conditional and the command are not fundamentally connected should use braces. +The following are correct uses: + +.. code:: cpp + + if (owns_cpu_data_ && h_data_) delete [] h_data_; // Ok. A method should only delete something if it is allocated and the class owns it. + if (x != 0) z = y / x; // Ok. Division by 0 is always invalid, regardless of code structure. + if (y % 2 == 1) { //Must be written this way, because x can be incremented regardless of y's parity. The logic is internal to our code. + x++; + } + + + Use of spaces and newlines ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/sphinx/developer_guide/git_guidelines.rst b/docs/sphinx/developer_guide/git_guidelines.rst index 73ae9f83c..cf90b5b73 100644 --- a/docs/sphinx/developer_guide/git_guidelines.rst +++ b/docs/sphinx/developer_guide/git_guidelines.rst @@ -1,4 +1,4 @@ -Developer Guidelines +Git Guidelines ==================== Branching and workflow @@ -29,37 +29,37 @@ When opening an issue, check that it is not a duplicate. Provide a clear and concise description, using the issue template. Pull Requests (PR)s -============ +------------ When opening a PR, make sure to select the correct base branch. If there are merge conflicts, use ``git rebase`` to resolve them before requesting a review. Detail changes made in your PR in the description. ------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1. What new features were added? 2. What bugs were fixed? 3. What tests were added? When reviewing a PR consider the following: ------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1. Does the PR address the issue? (if applicable) 2. Do existing and new tests pass? (Run the code on different machines) 3. Is the code clean, readable, and does it have proper comments? 4. Are the changes consistent with the coding guidelines? Minor concerns: ------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Add a comment to the PR with the minor concern and request the author to address it, but approve the merge. Major concern options: ------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1. Suggest a change with the Github suggest feature within the PR for the author to commit before merging. 2. Request the author to make the change and wait to approve the merge. 3. Branch off the PR, make the change, and submit a new PR with the change. Make the assignee the author of the current PR and request the author to merge the new PR. Merging ------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ All PRs must pass the checks and a review (by someone other than the PR author) before merging. Once the PR is approved and the checks pass, the author can merge the PR. diff --git a/docs/sphinx/developer_guide/index.rst b/docs/sphinx/developer_guide/index.rst index 24e137008..bec30dc07 100644 --- a/docs/sphinx/developer_guide/index.rst +++ b/docs/sphinx/developer_guide/index.rst @@ -1,100 +1,10 @@ -Building Re::Solve +Git and Documentation Tools ==================== -CMake Build System -------------------- - -Our ``cmake`` folder contains some basic CMake modules that help manage resolve: - -* ``cmake/FindKLU.cmake``: Our custom find module for KLU that we maintain -* ``cmake/ReSolveConfig.cmake.in``: Our custom config file that is used to generate the ``ReSolveConfig.cmake`` file that is installed with Re::Solve -* ``cmake/ReSolveFindCudaLibraries.cmake``: Our custom find module for CUDA libraries that we maintain to link in subset of cuda needed -* ``cmake/ReSolveFindHipLibraries.cmake``: Our custom find module for HIP/ROCm libraries that we maintain to link in subset of hip needed - -Apart from that check out our main ``CMakeLists.txt`` file for our remaining build configuration. - -We also export under the ``ReSolve::`` namespace in our installed CMake configuration for use with ``find_package`` as documented in our main ``README.md``. - -Spack Package ---------------- - -Our current Spack package has been introduced -`upstream `_, and contains support -for building Re::Solve with CUDA and HIP/ROCm support. - -We also have a custom ``spack`` folder/installation that contains our spack -submodule located in ``buildsystem/spack/spack``. This is used to build -Re::Solve on CI platforms, as well as support development of the spack package -as neccessary. - -See the Quick How-To section below for more information on how to update the -spack package and typical workflows for building Re::Solve with spack on CI -platforms for testing. - - -GitHub Actions ----------------- - -This is a quick summary of the workflows performed in each GitHub Action. For more information see the ``.github/workflows`` folder where each file is located. - -``documentation.yml`` -~~~~~~~~~~~~~~~~~~~~~~ - -``ornl_ascent_mirror.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Pushes to ORNL GitLab and triggers CI/CD pipelines there that are posted back to GitHub through commit messages. - -``ornl_crusher_mirror.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Pushes to ORNL Crusher GitLab... - -``pnnl_mirror.yml`` -~~~~~~~~~~~~~~~~~~~~ - -Pushes to PNNL GitLab... - -GitLab Pipelines ------------------ - -This is a quick summary of the workflows performed in each GitLab Pipeline. For more information see the ``yml`` file for each associated pipeline. - -``ornl/crusher.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Defines CI/CD for Crusher at ORNL. - -``.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~ - -Located in the root git directory, this defines the CI/CD pipelines for Ascent at ORNL. - -``pnnl/.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~ - -Since single GitLab repo triggers many pipelines as downstream dependents, we need a core config file to kick all of these builds off. - -``pnnl/base.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Describes core config shared across each job. This could be in ``pnnl/.gitlab-ci.yml`` but we keep it separate for clarity. - -``pnnl/deception.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Deception specific CI. - -``pnnl/incline.gitlab-ci.yml`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Incline specific CI. - - -Writing Documentation ---------------------- +Writing Sphynx Documentation +---------------------------- -Re::Solve uses Sphynx for the documentation. To write and preview the +Re::Solve uses Sphynx for online documentation. To write and preview the documentation on your local machine use e.g. ``pip`` tool to install following Python packages: @@ -178,6 +88,64 @@ Configures devcontainer through devcontainer features and sets up extensions. Small shell script that renders documentation and hosts it for quick development. +GitHub Actions +---------------------------- + +This is a quick summary of the workflows performed in each GitHub Action. For more information see the ``.github/workflows`` folder where each file is located. + +``documentation.yml`` +~~~~~~~~~~~~~~~~~~~~~~ + +``ornl_ascent_mirror.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Pushes to ORNL GitLab and triggers CI/CD pipelines there that are posted back to GitHub through commit messages. + +``ornl_crusher_mirror.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Pushes to ORNL Crusher GitLab... + +``pnnl_mirror.yml`` +~~~~~~~~~~~~~~~~~~~~ + +Pushes to PNNL GitLab... + +GitLab Pipelines +----------------- + +This is a quick summary of the workflows performed in each GitLab Pipeline. For more information see the ``yml`` file for each associated pipeline. + +``ornl/crusher.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Defines CI/CD for Crusher at ORNL. + +``.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~ + +Located in the root git directory, this defines the CI/CD pipelines for Ascent at ORNL. + +``pnnl/.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~ + +Since single GitLab repo triggers many pipelines as downstream dependents, we need a core config file to kick all of these builds off. + +``pnnl/base.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Describes core config shared across each job. This could be in ``pnnl/.gitlab-ci.yml`` but we keep it separate for clarity. + +``pnnl/deception.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Deception specific CI. + +``pnnl/incline.gitlab-ci.yml`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Incline specific CI. + Quick How-To guides ------------------- diff --git a/docs/sphinx/user_guide/index.rst b/docs/sphinx/user_guide/index.rst index c14225638..9bb6cf620 100644 --- a/docs/sphinx/user_guide/index.rst +++ b/docs/sphinx/user_guide/index.rst @@ -134,6 +134,40 @@ call the preset flag in the cmake build step. + +Advanced Builds +==================== + +CMake Build System +------------------- + +Our ``cmake`` folder contains some basic CMake modules that help manage resolve: + +* ``cmake/FindKLU.cmake``: Our custom find module for KLU that we maintain +* ``cmake/ReSolveConfig.cmake.in``: Our custom config file that is used to generate the ``ReSolveConfig.cmake`` file that is installed with Re::Solve +* ``cmake/ReSolveFindCudaLibraries.cmake``: Our custom find module for CUDA libraries that we maintain to link in subset of cuda needed +* ``cmake/ReSolveFindHipLibraries.cmake``: Our custom find module for HIP/ROCm libraries that we maintain to link in subset of hip needed + +Apart from that check out our main ``CMakeLists.txt`` file for our remaining build configuration. + +We also export under the ``ReSolve::`` namespace in our installed CMake configuration for use with ``find_package`` as documented in our main ``README.md``. + +Spack Package +--------------- + +Re::Solve can be built with +`spack `_, and contains support +for building Re::Solve with KLU and CUDA or HIP/ROCm support. + +We also have a custom ``spack`` folder/installation that contains our spack +submodule located in ``buildsystem/spack/spack``. This is used to build +Re::Solve on CI platforms, as well as support development of the spack package +as neccessary. + +See the Quick How-To section below for more information on how to update the +spack package and typical workflows for building Re::Solve with spack on CI +platforms for testing. + Getting Help ------------ From 8c3926b0fea4b29cf0c676b62198ae4a4161ae6b Mon Sep 17 00:00:00 2001 From: Shaked Regev Date: Wed, 4 Jun 2025 18:55:26 -0400 Subject: [PATCH 2/5] straggling change --- docs/doxygen/Doxyfile.in | 3 ++- docs/sphinx/developer_guide/index.rst | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/doxygen/Doxyfile.in b/docs/doxygen/Doxyfile.in index a9cd1fc34..b72ef9fc4 100644 --- a/docs/doxygen/Doxyfile.in +++ b/docs/doxygen/Doxyfile.in @@ -2787,4 +2787,5 @@ DOT_CLEANUP = YES # contain msc files that are included in the documentation (see the \mscfile # command). -MSCFILE_DIRS = \ No newline at end of file +MSCFILE_DIRS = +OUTPUT_DIRECTORY=../_readthedocs/html/doxygen \ No newline at end of file diff --git a/docs/sphinx/developer_guide/index.rst b/docs/sphinx/developer_guide/index.rst index bec30dc07..555792bf8 100644 --- a/docs/sphinx/developer_guide/index.rst +++ b/docs/sphinx/developer_guide/index.rst @@ -32,7 +32,7 @@ This will generate HTML documentation and place it in ``build`` subdirectory in your current directory. -Using Dev Container for Writing Documentation +Using Dev Containers for Writing Documentation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In case you cannot install Sphynx and other dependencies on your machine, From 91e29b31d6e4977a39cc1c6554492b2621757859 Mon Sep 17 00:00:00 2001 From: Shaked Regev Date: Thu, 5 Jun 2025 14:44:53 -0400 Subject: [PATCH 3/5] fixed typo --- docs/sphinx/developer_guide/coding_guidelines.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sphinx/developer_guide/coding_guidelines.rst b/docs/sphinx/developer_guide/coding_guidelines.rst index ada82e7f1..805e7e67a 100644 --- a/docs/sphinx/developer_guide/coding_guidelines.rst +++ b/docs/sphinx/developer_guide/coding_guidelines.rst @@ -93,7 +93,7 @@ where capitalization is important and it is preserved similarly. Capitalize variables refering to a matrix `A` such as `A`, `At`, `A_csc`, and `A_csr`. This is due to the equation $Ax=b$ and the Householder convention where -uppercase letters represent matrices and lowercase letter represent vectors +uppercase letters represent matrices and lowercase letter represent vectors. Pointers and references ~~~~~~~~~~~~~~~~~~~~~~~ From 6e8f6b402a484075ae25cc22161d84b57cef69a5 Mon Sep 17 00:00:00 2001 From: Adham Ibrahim <37982706+adham-ibrahim7@users.noreply.github.com> Date: Mon, 9 Jun 2025 08:43:28 -0400 Subject: [PATCH 4/5] fix typo --- docs/sphinx/user_guide/index.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sphinx/user_guide/index.rst b/docs/sphinx/user_guide/index.rst index 9bb6cf620..62eab0061 100644 --- a/docs/sphinx/user_guide/index.rst +++ b/docs/sphinx/user_guide/index.rst @@ -35,7 +35,7 @@ In the directory where you built the library run $ make install -To change the install location please use the CMAkePresets.json file as mentioned in `test and deploy <#test-and-deploy>`__ +To change the install location please use the CMakePresets.json file as mentioned in `test and deploy <#test-and-deploy>`__ To run it, download `test linear systems `__ and then edit script |runResolve|_ From 6a4eacfa6c8400ba31299980f6e8fb8ecd755139 Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Fri, 20 Jun 2025 21:40:51 -0400 Subject: [PATCH 5/5] Remove build artifacts from Doxyfile.in --- docs/doxygen/Doxyfile.in | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/doxygen/Doxyfile.in b/docs/doxygen/Doxyfile.in index b72ef9fc4..97a0b73af 100644 --- a/docs/doxygen/Doxyfile.in +++ b/docs/doxygen/Doxyfile.in @@ -2788,4 +2788,3 @@ DOT_CLEANUP = YES # command). MSCFILE_DIRS = -OUTPUT_DIRECTORY=../_readthedocs/html/doxygen \ No newline at end of file