<p><b>ringler@lanl.gov</b> 2011-02-08 12:53:07 -0700 (Tue, 08 Feb 2011)</p><p><br>
added reviewer comments to be considered before sending module to entire developer list for consideration.<br>
</p><hr noshade><pre><font color="gray">Modified: branches/ocean_projects/vert_adv_mrp/src/operators/module_spline_interpolation.F
===================================================================
--- branches/ocean_projects/vert_adv_mrp/src/operators/module_spline_interpolation.F        2011-02-08 16:40:00 UTC (rev 717)
+++ branches/ocean_projects/vert_adv_mrp/src/operators/module_spline_interpolation.F        2011-02-08 19:53:07 UTC (rev 718)
@@ -1,3 +1,14 @@
+!TDR: grep for "TDR" to find specific comments
+!global suggestions (see specific comments for reasoning)
+!        zIn goes to xIn
+!        zOut goes to xOut
+!        better explanation of subroutines
+!        remove "ocean specific" nomenclature (like "layer thickness")
+
+! TDR: I am happy to implement these changes, but it would be better
+! for you to do it, then I can review it.
+
+
module spline_interpolation
implicit none
@@ -9,10 +20,31 @@
IntegrateColumnCubicSpline, InterpolateColumnLinear, &
TestInterpolateColumn
+! TDR: is there a general reference for this entire module?
+! TDR: provide a brief explanation of what each public interface does
+! TDR: i.e. what is a typical use of each subroutine?
+! TDR: e.g. : given data (y) at locations (x), determine spline coefficients that can be passed into SSS.
+
+!        CubicSplineCoefficients:
+
+!        InterpolateCubicSpline:
+
+!        InterpolateColumnCubicSpline:
+
+!        IntegrateCubicSpline:
+
+!        IntegrateColumnCubicSpline:
+
+!        InterpolateColumnLinear:
+
+!        TestInterpolateColumn:
+
contains
subroutine CubicSplineCoefficients(x,y,n,yp1,ypn,y2)
+! TDR: The below description is pretty confusing
+
! Given n pairs (x_i, y_i), compute values of y2, which are
! dy^2/dx^2 at each node used to create a cubic spline on each
! interval. yp1 and ypn are the first derivatives at each end,
@@ -25,6 +57,8 @@
integer, intent(in) :: &
n ! number of nodes
real(kind=RKIND), intent(in), dimension(n) :: &
+!TDR: since this will be in frameworks, there should be no
+!TDR "core specific" annotation. I suggest "location of nodes" throughout
x, &! depth of nodes
y ! value at nodes
real(kind=RKIND), intent(in) :: &
@@ -80,6 +114,8 @@
subroutine InterpolateCubicSpline(xa,ya,y2a,n,x,y)
+! TDR: this description is pretty confusing.
+
! Given arrays xa and ya of length n, which tabulate a function (with
! xa in order), and given the array y2a, which is the output from spline,
! and given a value of x this routine returns a cubic-spline interpolated
@@ -133,6 +169,12 @@
zIn, zOut, &
kmaxIn, kmaxOut)
+! TDR: this description is better, but we need to refrain from
+! TDR using core specific language. why switch to zIn/zOut. I think
+! TDR: that we should use xIn/xOut to be consistent with the other
+! TDR: subroutines. In that, zIn is replaced by xIn and is described
+! TDR: as "x locations of nodes, assumed to be monotonically increasing"
+
! Given kmaxIn pairs (zIn,phiIn), compute cubic spline interpolation
! phiOut at each zOut.
! This subroutine assumes that both zIn and zOut are monotonically
@@ -347,6 +389,8 @@
zIn,zOut, &
kmaxIn,kmaxOut)
+! TDR: Desciptions need to be "core independent"
+
! !DESCRIPTION:
! Given kmaxIn pairs (zIn,phiIn), compute the linear interpolation
! phiOut at each zOut.
</font>
</pre>