Fixing some doxygen documentation - first pass #918
Conversation
doxygenated module_wrt_grid_comp.F90
turn on more doxygen warnings
more doxygen
Doxygenate fv_moving_nest_types.F90 and fv_moving_nest_physics
Moving doxygen changes from old PR to ejh_doxygen branch
edwardhartnett
changed the title
Complete doxygen for moving_nest subdirectory
Doxygen for cpl subdirectory
Update fv3_cap.F90
| @@ -1,3 +1,6 @@ | |||
| !> @file | |||
| !> @brief ??? | |||
There was a problem hiding this comment.
@rmontuoro can you please fill in what all the "???" documentation should be in this file? According to github, you originally added this file.
|
|
||
| !> ??? | ||
| !> | ||
| !> @param[in] gcomp ??? |
There was a problem hiding this comment.
@junwang-noaa can you please tell us what the ??? should be in this documentation?
| integer, public :: nvar_before_lake = 0 | ||
|
|
||
| ! The lsoil flag is only meaningful when reading:; | ||
| integer, public :: nvar2o = 0 !< ??? |
There was a problem hiding this comment.
@SamuelTrahanNOAA can you please tell us what the ??? in this documentation should be?
| @@ -64,7 +70,12 @@ subroutine fill_bbox_r4_2d(out_bbox, in_grid) | |||
| out_bbox%je = ubound(in_grid, 2) | |||
| end subroutine fill_bbox_r4_2d | |||
|
|
|||
|
|
|||
| !> ??? | |||
There was a problem hiding this comment.
@wramstrom can you please tell us what should replace the ??? in this documentation?
| real(kind=kind_phys), dimension(:,:,:), allocatable, save :: smc | ||
| real(kind=kind_phys), dimension(:,:,:), allocatable, save :: stc | ||
| real(kind=kind_phys), dimension(:,:,:), allocatable, save :: slc | ||
| real(kind=kind_phys), dimension(:,:), allocatable, save :: xlat !< ??? |
There was a problem hiding this comment.
@climbfuji can you tell us what should replace the ??? in this documentation?
There was a problem hiding this comment.
I would check GFS_typedefs.F90/GFS_typedefs.meta for the variable with the same name
There was a problem hiding this comment.
This is your code right?
There was a problem hiding this comment.
I wrote some of this many years ago, then @pjpegion and @lisa-bengtsson updated some of it, and recently I had to modify those fields when moving from blocked data structures to contiguous arrays. The code further down in the file unambiguously relates these local variables with variables in GFS_typedefs.F90, for example:
call transfer_field_to_stochastics(GFS_Control%blksz, GFS_Grid%xlat, xlat)
In GFS_typedefs.F90,
fv3atm/ccpp/data/GFS_typedefs.F90
Lines 1694 to 1700 in 2c7357b
Thus, xlat here should be the same, I suppose.
!< grid latitude in radians, default to pi/2 ->
!! -pi/2 range, otherwise adj in subr called
There was a problem hiding this comment.
I think all module variables in this file can be (and probably should be) declared as private. Doxygen should not complain about undocumented private (and local) variables.
There was a problem hiding this comment.
Private and public module vars need to be documented.
There was a problem hiding this comment.
I'm sure we all agree that this is important code and should be documented. In addition, NCO has documentation requirements which are not being met by this code.
@climbfuji since you are the original author, could you do a PR against the ejh_doxygen branch with the missing documentation from the module variables and all of the subroutines?
Presumably the meaning of this code is obvious to you, but that needs to be documented. We are intending to write unit tests for the code in fv3atm, and that will be mostly impossible without documentation of the code. Since you have an understanding of what this code is, that shouldn't take more than a few minutes of your time...
There was a problem hiding this comment.
Can you provide a link to NCO's documentation requirements document where the requirement that all private and local variables are documented using doxygen is specified?
There was a problem hiding this comment.
I'm sure we all agree that this is important code and should be documented. In addition, NCO has documentation requirements which are not being met by this code.
@climbfuji since you are the original author, could you do a PR against the ejh_doxygen branch with the missing documentation from the module variables and all of the subroutines?
Presumably the meaning of this code is obvious to you, but that needs to be documented. We are intending to write unit tests for the code in fv3atm, and that will be mostly impossible without documentation of the code. Since you have an understanding of what this code is, that shouldn't take more than a few minutes of your time...
I won't have time for this for several weeks if not months, sorry. It's all in GFS_typedefs and can be taken from there if someone wants to do this in the meantime.
Description
Fixing some doxygen documentation
Issue(s) addressed
Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)
Testing
How were these changes tested?
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?
Dependencies
If testing this branch requires non-default branches in other repositories, list them.
Those branches should have matching names (ideally)