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

[WIP] Integrating Doxygen/Breathe with TECA's rtd #505

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

elbashandy
Copy link
Contributor

Resolves #478

@elbashandy
Copy link
Contributor Author

I found a library called Exhale that converts Doxygen generated documentation into Sphinx docs using Breathe. However, after playing with Exhale, I realized that it might not be the right tool for us because it is pretty sensitive to any Doxygen errors, and TECA isn't fully documented properly in the style that Doxygen expects.

I think that Breathe might be the right choice to begin with as we can selectively choose the Algorithms & the software utilities that we want to expose in the documentation (Exhale tries to show everything.)

To show an example of what Doxygen & Breathe generate, I hosted some static pages of what I have in mind on how TECA should showcase the framework elements. This is still an early stage and it's not fully documented yet. You will find that I pretty much followed TECA's current organizational scheme [alg, core, data, io, system].

https://elbashandy.github.io/html/framework/framework.html

@elbashandy
Copy link
Contributor Author

elbashandy commented Dec 17, 2020

Creating a list of files that need to conform to Doxygen's documentation style:

Algorithms:

  • teca_2d_component_area
  • teca_apply_binary_mask
  • teca_ar_detect
  • teca_bayesian_ar_detect
  • teca_bayesian_ar_detect_parameters
  • teca_binary_segmentation
  • teca_cartesian_mesh_regrid
  • teca_cartesian_mesh_source
  • teca_cartesian_mesh_subset
  • teca_component_area_filter
  • teca_component_statistics
  • teca_connected_components
  • teca_dataset_capture
  • teca_dataset_diff
  • teca_dataset_source
  • teca_deeplab_ar_detect
  • teca_derived_quantity
  • teca_derived_quantity_numerics
  • teca_descriptive_statistics
  • teca_distance_function
  • teca_evaluate_expression
  • teca_face_to_cell_centering
  • teca_geography
  • teca_geometry
  • teca_integrated_vapor_transport
  • teca_l2_norm
  • teca_laplacian
  • teca_latitude_damper
  • teca_mask
  • teca_model_segmentation
  • teca_normalize_coordinates
  • teca_parser
  • teca_python_algorithm
  • teca_python_reduce
  • teca_saffir_simpson
  • teca_table_calendar
  • teca_table_reduce
  • teca_table_region_mask
  • teca_table_remove_rows
  • teca_table_sort
  • teca_table_to_stream
  • teca_tc_activity
  • teca_tc_candidates
  • teca_tc_classify
  • teca_tc_stats
  • teca_tc_trajectory
  • teca_tc_trajectory_scalars
  • teca_tc_wind_radii
  • teca_tc_wind_radii_stats
  • teca_temporal_average
  • teca_temporal_reduction
  • teca_threaded_python_algorithm
  • teca_variant_array_operand
  • teca_variant_array_operator
  • teca_vertical_coordinate_transform
  • teca_vertical_reduction
  • teca_vorticity

Core:

  • teca_algorithm
  • teca_algorithm_executive
  • teca_algorithm_fwd
  • teca_algorithm_output_port
  • teca_binary_stream
  • teca_calendar
  • teca_common
  • teca_dataset
  • teca_dataset_fwd
  • teca_index_executive
  • teca_index_reduce
  • teca_index_reduce_fwd
  • teca_memory_profiler
  • teca_metadata
  • teca_metadata_util
  • teca_mpi
  • teca_mpi_manager
  • teca_output_port
  • teca_parallel_id
  • teca_profiler
  • teca_program_options
  • teca_programmable_algorithm
  • teca_programmable_algorithm_fwd
  • teca_programmable_reduce
  • teca_programmable_reduce_fwd
  • teca_shared_object
  • teca_string_util
  • teca_system_util
  • teca_thread_pool
  • teca_thread_util
  • teca_threaded_algorithm
  • teca_threaded_algorithm_fwd
  • teca_threaded_programmable_algorithm
  • teca_threadsafe_queue
  • teca_type_select
  • teca_uuid
  • teca_variant_array
  • teca_variant_array_fwd

Data

  • teca_arakawa_c_grid
  • teca_array_attributes
  • teca_array_collection
  • teca_array_collection_fwd
  • teca_cartesian_mesh
  • teca_coordinate_util
  • teca_curvilinear_mesh
  • teca_database
  • teca_database_fwd
  • teca_dataset_util
  • teca_mesh
  • teca_mesh_fwd
  • teca_table
  • teca_table_collection
  • teca_table_collection_fwd
  • teca_table_fwd
  • teca_uniform_cartesian_mesh
  • teca_uniform_cartesian_mesh_fwd

I/O

  • teca_cartesian_mesh_reader
  • teca_cartesian_mesh_reader_factory
  • teca_cartesian_mesh_writer
  • teca_cartesian_mesh_writer_factory
  • teca_cf_layout_manager
  • teca_cf_reader
  • teca_cf_time_step_mapper
  • teca_cf_writer
  • teca_file_util
  • teca_multi_cf_reader
  • teca_netcdf_util
  • teca_table_reader
  • teca_table_writer
  • teca_vtk_util
  • teca_wrf_reader

@elbashandy
Copy link
Contributor Author

elbashandy commented Dec 17, 2020

@taobrienlbl @burlen feel free to modify the list that I documented above. I am going to go over this list one by one to make sure that the code documentation is picked up by Doxygen.

@elbashandy
Copy link
Contributor Author

@taobrienlbl @burlen I want to take your opinion to discuss several approaches to choose from on how to document TECA macro expanded functions.

The 1st approach is a basic one which I manually specify the function names that I want to document:

/*! \fn set_component_variable(const std::string &v)

       set the name of the input array.
*/
/*! \fn get_component_variable() const

       get the name of the input array.
*/
TECA_ALGORITHM_PROPERTY(std::string, component_variable)

or we can modify the macro definition so that we can add a comment block above each generated function.
2nd approach:

#define TECA_ALGORITHM_PROPERTY(T, NAME, DESCRIPTION) \
                                                      \
/*! set DESCRIPTION */                                \
void set_##NAME(const T &v)                           \
{                                                     \
    if (this->NAME != v)                              \
    {                                                 \
        this->NAME = v;                               \
        this->set_modified();                         \
    }                                                 \
}                                                     \
                                                      \
/*! get DESCRIPTION */                                \
const T &get_##NAME() const                           \
{                                                     \
    return this->NAME;                                \
}

TECA_ALGORITHM_PROPERTY(std::string, component_variable,
        the name of the input array.)

I am leaning towards the 2nd approach as it is much cleaner and straightforward. However, it is less flexible because it uses the same description body (Unless of course we add an additional parameter to the macro - TECA_ALGORITHM_PROPERTY(T, NAME, DESCRIPTION_1, DESCRIPTION_2)).

Whatever approach we are going to use is going to take some effort to apply to all files. So hearing your opinion will be very much appreciated before I commit to one approach. Thanks :)

@burlen
Copy link
Collaborator

burlen commented Dec 18, 2020

@taobrienlbl @burlen I want to take your opinion to discuss several approaches to choose from on how to document TECA macro expanded functions.

The 1st approach is a basic one which I manually specify the function names that I want to document:
...
or we can modify the macro definition so that we can add a comment block above each generated function.
...

With regard to the TECA_ALGORITHM_PROPERTY and TECA_ALGORITHM_VECTOR_PROPERTY macros, I would like to suggest the following alternative.

Annotate each function in the macro with a terse description of what the function does using the algorithm property name as passed into the macro. Then use the grouping mechanism to provide the main documentation for the property. Include an anchor so that the property may be linked to elsewhere.

Also please use the so called javadoc /** */ style doxygen markup.

An illustrative example follows:

#ifndef doxtest_h
#define doxtest_h

#define SET(_T, _N)                             \
    /** set the @ref _N algorithm property. */  \
    void set_  ## _N ( _T v)                    \
    { this-> _N = v; }                          \
                                                \
    /** get the @ref _N algorithm property. */  \
    _T get_ ## _N ()                            \
    { return this-> _N; }

/// Solves A*x = b via Jaccobi iteration.
/**
 * doxtest is a class that lets us try different doxygen documentation formats.
 * it's for our own internal use.  this is a long and rambling description.
 *
 * the @ref error_tolerance and @ref max_iter  properties control the solver.
*/
class doxtest
{
public:
    doxtest();
    ~doxtest();

    /** @anchor error_tolerance
     * @name error_tolerance
     * Sets the tolerance used for solver convergence. When the error
     * between two successive iterations is less than or equal to this value
     * then solver execution is stopped.
     */
    ///@{
    SET(double, error_tolerance)
    ///@}

    /** @anchor max_iter
     * @name max_iter
     * Sets the maximum number of sover iterations. When this number of                                                                                                                                                                                                                      
     * iteration is reached before the desired error tolerance is acheived the
     * solver will error out.
     */
    ///@{
    SET(int, max_iter)
    ///@}

    /** run the solver. if the solver converges to error_tolerance in less than max_iter iterations
     * then the solve was successful. Returns 0 if successful and non-zero otherwise.
     */
    int solve();

    /** print the class parameters.
     * sends the values to stderr
     */
    void print();

private:
    double error_tolerance;
    int max_iter;
};

#endif

and this is what it looks like
image

what does @taobrienlbl think?

@elbashandy
Copy link
Contributor Author

@burlen great suggestion! Thank you.

Copy link
Collaborator

@burlen burlen left a comment

Choose a reason for hiding this comment

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

Hi Abdel, Since you'll be working on getting this work in, I went ahead and marked a few things
that I saw.

@@ -32,6 +32,7 @@ std::shared_ptr<T const> shared_from_this() const \
}

#define TECA_ALGORITHM_CLASS_NAME(T) \
/** returns the string value "T" */ \
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a virtual method and can be overriden in dervied classes. it does not necessarily return "T". say instead: returns the name of the class.

} \
} \
\
/** Get the @ref NAME algorithm property */ \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get the value of the ...

return this->NAME; \
#define TECA_ALGORITHM_PROPERTY(T, NAME) \
\
/** Set the @ref NAME algorithm property */ \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Set the value of the ...

| | | i.e. the background. This can be used to skip processing |
| | | of the background when desirable. |
+----------------------------+------------------------------------------------------------+
@endrst
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sold on the idea of using rst tables for this. What's the reason? Was it to test with Breathe?

Also, use @ref component_variable etc to make a link to the property's documentation

@@ -57,18 +73,23 @@ class teca_2d_component_area : public teca_algorithm
TECA_GET_ALGORITHM_PROPERTIES_DESCRIPTION()
TECA_SET_ALGORITHM_PROPERTIES()

// set the name of the input array
// The component variable name containing the region labels.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use doxygen style comments and grouping here as well.

The documentation should read something to the effect:

Set/get the name of the variable  containing the connected component labels

Comment on lines +57 to 76
/** list the interpolation modes used in transfering
* data between meshes of differing resolution.
* In nearest mode value at the nearest grid point
* is used, in linear mode bi/tri linear interpolation
* is used.
*/
enum
{
/** 0 */
nearest=0,
/** 1 */
linear=1
};

/** @anchor interpolation_mode
* @name interpolation_mode
* Set the interpolation_mode mode. default is nearest
*/
///@{
TECA_ALGORITHM_PROPERTY(int, interpolation_mode)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see my comments about threshold_mode above, all the same applies here

// 0 otherwise
/** @anchor tolerance
* @name tolerance
* Set tolerance. Relative tolerance below which two floating-point
Copy link
Collaborator

Choose a reason for hiding this comment

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

Set the relative tolerance ...

Note: this class has changed substantially and you might be better off copying the things you want to keep than trying to resolve conflicts

@@ -101,7 +114,7 @@ class teca_dataset_diff : public teca_algorithm
const teca_metadata &request) override;

private:
// Tolerance for equality of field values.
/** Tolerance for equality of field values. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is documented via the algorithm property, we should probably remove this one

// default is 32 bit floating point. Use
// teca_variant_array_type<NT>::get() to get specific type
// codes for C++ POD types NT.
/** @anchor coordinate_type_code
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ooops. I also made major revisions to teca_cartesian_mesh_source class and its documentation. I've added and revised the documentation in addition to the stylistic changes you made. To avoid losing the new text during conflict resolution, please drop these changes.

@@ -15,10 +15,11 @@ largest scale on DOE HPC systems.
Storm Tracks Generated by TECA

.. toctree::
:maxdepth: 3
:maxdepth: 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless you have a compelling reason for the change I'd prefer to leave at 3 for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup doxygen and display it in read the docs
2 participants