math icon indicating copy to clipboard operation
math copied to clipboard

[WIP] Fix doxygen so it compiles with 0 errors and update docs

Open SteveBronder opened this issue 2 years ago • 13 comments

Summary

This cleans up a lot of the doxygen to remove a lot of the warnings we previously had. The biggest warning was about parameters being defined multiple times for different overloads. This was caused by doxygen parsing functions while ignoring non-type template parameters which we use for writing our overloads. For example, doxygen would look at both of these functions signatures as if they were the same

template <typename T, require_arithmetic_t<T>* = nullptr>
auto a_fun(const T& x);

template <typename T, require_var_t<T>* = nullptr>
auto a_fun(const T& x);

To fix this I just changed the names of the template parameters. When possible I would change the name to reflect, conceptually, what types would go into the overload. But in some cases there are several generic overloads and so for those I just added arbitrary named templates like T1, T2, T3, etc.

Tests

This should only be a cosmetic change so no new tests should be necessary.

The docs should be looked over from a local build of the website using make doxygen

Side Effects

Yes, doxygen now requires at least version 1.8.13. This is the default for Ubuntu in 20.04, but I'm not sure what version homebrew has. I also got this to make the docs with doxygen 1.9.1 but there could be errors I'm missing with newer versions like 1.9.4. Bullseye Debian only has version 1.8.3 so our docs will not compile on Bullseye debian systems with the basic install unless they install doxygen from backports.

Release notes

Update the documentation

Checklist

  • [ ] Math issue #(issue number)

  • [x] Copyright holder: Steve Bronder

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses: - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause) - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • [x] the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • [x] the code is written in idiomatic C++ and changes are documented in the doxygen

  • [x] the new changes are tested

SteveBronder avatar May 24 '22 22:05 SteveBronder

@bob-carpenter I still need to finish some of the edits to the adding new functions documentation but this should be ready to read over by tmrw or so

SteveBronder avatar May 24 '22 22:05 SteveBronder

Alrighty @bob-carpenter I think I got everything sorted out. Can you try checking out this branch and running make doxygen? If you have time to look over the section on adding new functions I think I got most of the pieces you were had Qs about

SteveBronder avatar Jun 01 '22 19:06 SteveBronder

The branch doesn't build for me in doxygen. Could I have the wrong version of doxygen installed?

~/github/stan-dev/cmdstan/stan/lib/stan_math (fix/docs1)$ doxygen --version
1.9.3

Here's the error dump from doxygen:

~/github/stan-dev/cmdstan/stan/lib/stan_math (fix/docs1)$ make -j8 doxygen
mkdir -p doc/api
doxygen -v
1.9.3
doxygen doxygen/doxygen.cfg
warning: Tag 'OUTPUT_TEXT_DIRECTION' at line 102 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'CLANG_ASSISTED_PARSING' at line 1114 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_ADD_INC_PATHS' at line 1120 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_OPTIONS' at line 1128 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_DATABASE_PATH' at line 1174 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'LATEX_SOURCE_CODE' at line 1941 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'RTF_SOURCE_CODE' at line 2031 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOCBOOK_PROGRAMLISTING' at line 2136 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'CLASS_DIAGRAMS' at line 2340 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
/Users/bcarpenter/github/stan-dev/cmdstan/stan/lib/stan_math/lib/opencl_3.0.0/CL/opencl.hpp:2061: error: the \endcond does not have a corresponding \cond in this file (warning treated as error, aborting now)
make: *** [doxygen] Error 1

bob-carpenter avatar Jun 01 '22 21:06 bob-carpenter

No this is an error when compiling with 1.9.1+, it is trying to document the OpenCL library we use as well which is odd. Let me see if I can tell it to not try to do that. Oddly this doesn't show up for <= 1.9.1

SteveBronder avatar Jun 01 '22 23:06 SteveBronder

Alrighty @bob-carpenter can you give it another shot?

SteveBronder avatar Jun 02 '22 15:06 SteveBronder

Awesome. It works now with only some config warnings. Should I go ahead and review and merge?

~/github/stan-dev/cmdstan/stan/lib/stan_math (fix/docs1)$ make -j8 doxygen
mkdir -p doc/api
doxygen -v
1.9.3
doxygen doxygen/doxygen.cfg
warning: Tag 'OUTPUT_TEXT_DIRECTION' at line 102 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'CLANG_ASSISTED_PARSING' at line 1114 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_ADD_INC_PATHS' at line 1120 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_OPTIONS' at line 1128 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'CLANG_DATABASE_PATH' at line 1174 of file 'doxygen/doxygen.cfg' belongs to an option that was not enabled at compile time.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled.
warning: Tag 'LATEX_SOURCE_CODE' at line 1941 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'RTF_SOURCE_CODE' at line 2031 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOCBOOK_PROGRAMLISTING' at line 2136 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'CLASS_DIAGRAMS' at line 2340 of file 'doxygen/doxygen.cfg' has become obsolete.
         To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
cp ./doxygen/pretty_stuff/eigen_navtree_hacks.js ./doc/api/html
~/github/stan-dev/cmdstan/stan/lib/stan_math (fix/docs1)$ 

bob-carpenter avatar Jun 02 '22 16:06 bob-carpenter

If you can start reviewing the doxygen docs I wrote that would be great, but I need to check out what the error is coming from on Jenkins right now in prim

SteveBronder avatar Jun 02 '22 19:06 SteveBronder

It looks like the error may be on Jenkins config.

I don't know how to mechanically do the review here. The 400+ files just chokes my web browser and the web page takes forever to scroll (30+ seconds) or to click (60+ seconds). Suggestions?

bob-carpenter avatar Jun 02 '22 20:06 bob-carpenter

Oh wow, so on my desktop its fine but on my laptop it totally crashes. Can you try the link below while also setting the diff to unified view?

image

https://github.com/stan-dev/math/pull/2745/files?file-filters%5B%5D=.cfg&file-filters%5B%5D=.css&file-filters%5B%5D=.h&file-filters%5B%5D=.html&file-filters%5B%5D=.js&file-filters%5B%5D=.md&file-filters%5B%5D=.xml&show-viewed-files=true&w=1

Both of those things managed to get the docs up and reviewable. I think unified view also works for reviewing the code

SteveBronder avatar Jun 03 '22 03:06 SteveBronder

Thanks, @SteveBronder---that works. Now onto the actual reviewing.

bob-carpenter avatar Jun 03 '22 16:06 bob-carpenter

Thank you for the thorough review! Yes it took quite a while to write but I think it should help folks in the future. I'll give this a look over the weekend

SteveBronder avatar Jun 03 '22 21:06 SteveBronder

Seriously, you can take as much of that feedback as you want. It's great as it is, and I wouldn't want to block it on grammar, formatting, etc.

bob-carpenter avatar Jun 09 '22 17:06 bob-carpenter

Alright I think I got everything! At this point just need to do a find replace to add newlines after each sentence and remove some of the I / we usage. @bob-carpenter I'll ping you when I'm finished with those

SteveBronder avatar Jun 24 '22 19:06 SteveBronder

@SteveBronder, I'm trying to resurrect this PR. I merged develop into it to the best of my ability.

syclik avatar Jan 03 '23 21:01 syclik