Skip to content

Updated documentation for horiz_interp - #1900

Open
mlee03 wants to merge 17 commits into
NOAA-GFDL:mainfrom
mlee03:horiz_interp_doc
Open

Updated documentation for horiz_interp#1900
mlee03 wants to merge 17 commits into
NOAA-GFDL:mainfrom
mlee03:horiz_interp_doc

Conversation

@mlee03

@mlee03 mlee03 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description
This PR adds Doxygen documentation and a Quickstart guide to horiz_interp_mod.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

Comment thread horiz_interp/QUICKSTART.md
Comment thread horiz_interp/include/horiz_interp.inc
!> @parblock
!! Unused subroutine.
!! @endparblock
subroutine FILL_XY_(fi, ics, ice, jcs, jce, mask, maxpass)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

check if used in FMS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not used in FMS~

Comment thread horiz_interp/include/horiz_interp_bilinear.inc Outdated
Comment thread horiz_interp/horiz_interp.F90
Comment thread horiz_interp/horiz_interp.F90
Comment thread horiz_interp/horiz_interp_bicubic.F90 Outdated
end interface

!> find the lower neighbour of xf in field xc, return is the index
!> Unused generic interface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Check if used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's used!

Comment thread horiz_interp/horiz_interp_bilinear.F90 Outdated
Comment thread horiz_interp/horiz_interp_spherical.F90 Outdated
Comment thread horiz_interp/horiz_interp_type.F90 Outdated
@mlee03
mlee03 marked this pull request as ready for review August 6, 2026 19:23
@mlee03
mlee03 requested a lite review from Copilot August 6, 2026 19:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (18)

horiz_interp/QUICKSTART.md:74

  • The blackbox example calls horiz_interp_end(), but it isn’t imported in the "use horiz_interp_mod, only:" list.
use horiz_interp_mod, only: horiz_interp_init, horiz_interp, horiz_interp_del

horiz_interp/QUICKSTART.md:6

  • Typo: "longititude" should be "longitude" (and this sentence is describing lon/lat coordinates).
grids, specified in longititude and latitude, must be in radians.

horiz_interp/QUICKSTART.md:29

  • The example calls horiz_interp_end(), but it is not imported in the "use horiz_interp_mod, only:" list, so the snippet won’t compile as written.

This issue also appears on line 74 of the same file.

use horiz_interp_mod, only: horiz_interp_init, horiz_interp_new, &
                            horiz_interp, horiz_interp_del, horiz_interp_type

horiz_interp/QUICKSTART.md:58

  • Typo in comment: "Inerp" should be "Interp".
! deallocate memory stored in Inerp

horiz_interp/QUICKSTART.md:132

  • The 1D-destination-grid snippet won’t compile with implicit none: the implied-do variable i is undeclared, lon_dst/lat_dst are allocatable but never allocated, and the array-constructor implied-do syntax needs parentheses. Also dl is undefined.
lon_dst = [real(i, r8_kind)*DEG_TO_RAD, i=1, nlon_dst]
lat_dst = [dl*real(i, r8_kind)*DEG_TO_RAD, i=-nlat_dst/2, nlat_dst/2]

horiz_interp/QUICKSTART.md:209

  • The documented interp_method value "conserve" does not match the implementation, which selects on "conservative" (see horiz_interp/include/horiz_interp.inc). Also “Supports all combination” should be “combinations”.
* `"conserve"` — order 1 conservative interpolation.  Supports all combination of 1D and 2D source and destination grids.

horiz_interp/QUICKSTART.md:257

  • Interp has an integer member %version (not %version1); this sentence should reference Interp%version == 1 for version-1 conservative weights.
* `mask_in`/ `mask_out`:  Used only when `Interp%version1 = .true.` (when weights were generated from 1D representation of both input and output grids.)

horiz_interp/QUICKSTART.md:276

  • Typo: "supporst" should be "supports".
- Horiz_interp_mod supporst both 32-bit (`r4_kind`) and 64-bit (`r8_kind`) grids and data.

horiz_interp/include/horiz_interp.inc:174

  • Typo in Doxygen block: "retangular" should be "rectangular".
  !! interpolation for retangular input grids specified as 1D arrays and 2D output grids

horiz_interp/include/horiz_interp.inc:188

  • The parameter documentation for lon_in/lat_in is swapped: lon_in should describe longitude and lat_in should describe latitude.
   real(FMS_HI_KIND_), intent(in),  dimension(:) :: lon_in
     !< are the latitude coordinates [radians] for the rectangular input grid
   real(FMS_HI_KIND_), intent(in),  dimension(:) :: lat_in
     !< are the longitude coordinates [radians] for the rectangular input grid

horiz_interp/include/horiz_interp.inc:192

  • The parameter documentation for lon_out/lat_out is swapped: lon_out should describe longitude and lat_out should describe latitude.
   real(FMS_HI_KIND_), intent(in),  dimension(:,:) :: lon_out
     !< are the 2D latitude coordinates [radians] for the output grid
   real(FMS_HI_KIND_), intent(in),  dimension(:,:) :: lat_out
     !< are the 2D longitude coordinates [radians] for the output grid

horiz_interp/include/horiz_interp_conserve_xgrid.c:33

  • The @file Doxygen comment is not terminated correctly ("* /"), which causes the following comment block to be swallowed into the file header docs.
/** @file
 *  @ingroup horiz_interp_conserve_mod
 *  @brief Functions to compute the exchange grid
 * /

horiz_interp/horiz_interp.F90:266

  • Doxygen tag typo: "@parblcok" should be "@parblock" so Doxygen recognizes the block.
 !> @parblcok

horiz_interp/horiz_interp_bilinear.F90:100

  • Doxygen tag typo: "@parbock" should be "@parblock" so Doxygen recognizes the block.
  !> @parbock
  !! Initializes horiz_interp_bilinear_mod.  Called from horiz_interp_init in horiz_interp_mod.
  !! @endparblock

horiz_interp/horiz_interp_bilinear.F90:115

  • Typo: "Caled" should be "Called".
  !! Resets %is_allocated to .false.  Caled from horiz_interp_del in horiz_interp_mod.

horiz_interp/horiz_interp_bicubic.F90:139

  • Doxygen tag typo: "@parblcok" should be "@parblock" so Doxygen recognizes the block.
  !> @parblcok

horiz_interp/horiz_interp_type.F90:181

  • The comments describing the nested real-kind storage types are swapped: Reals8 is 64-bit and Reals4 is 32-bit (also “floating pointer” should be “floating point”).
   type(horizInterpReals8_type) :: horizInterpReals8_type
     !< holds more 32-bit floating point data required for interpolation.
   type(horizInterpReals4_type) :: horizInterpReals4_type
     !< holds more 64-bit floating pointer data required for interpolation.

horiz_interp/horiz_interp_type.F90:191

  • Doxygen command should be "@endparblock" without trailing punctuation; the dot can prevent Doxygen from recognizing the end of the block.
  !! @endparblock.

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.

2 participants