PyAOS / aoslib

A collection of standard atmospheric and oceanic sciences routines.
45 stars 32 forks source link

Wrapping steps #6

Open jwblin opened 11 years ago

jwblin commented 11 years ago

Jonathan H. wrote: "In the PR I tried to give an example of what needs to be done using the calctd rountine. The steps were:

  1. Adjust the f2py signature for the subroutine to properly reflect the input, output and hidden parameters.
  2. Write a Python function which calls the wrapped function.
  3. Create a docstring consistent with the Numpy/SciPy docstring standard, much of this can be modified from documentation in the Fortran file. (https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt)
  4. Create a test function (or class) which can run with nose+unittest
  5. Add the documentation and rebuild.

This was an iterative/parallel process and some of the detail should be discussed before we do start performing this for all the routines in the library."

Thoughts from folks?

jwblin commented 11 years ago

Interface questions with my thoughts:

(1) Should functions use kwargs or args?: I'm partial to the convention of args for required input and kwd args for optional input.

(2) Do we want to add parameter checking: I'm a little torn on this. Part of me thinks for the first wrapped functions, we don't need to add parameter checking, since (in my mind) it's to get a first cut at a set of functions that are validated by tests and whose results can then be a comparison set for later versions of those functions (i.e., that are re-written in NumPy to be rank agnostic). On the other hand, these first wrapped functions will be more usable with parameter checking.

(3) Where should the documentation go: I'd say docstrings for all API documentation and a separate users guide for more "how to" documentation.

jjhelmus commented 11 years ago

A note on point (2): By using well defined f2py signatures such as the one for calctd

        subroutine calctd(t,rh,mni,ni,nj,td) ! in calctd.f
            real dimension(mni,nj), intent(in) :: t
            real dimension(mni,nj), intent(in) :: rh
            integer intent(hide), depend(t) :: mni=shape(t,0)
            integer intent(hide), depend(t) :: nj=shape(t,1)
            integer optional, check(ni<=shape(t,0)) :: ni=shape(t,0)
            real dimension(mni,nj), intent(out), depend(mni, nj) :: td
        end subroutine calctd

The Python front end can be very simple yet we still get some basic parameter recasting and checking.

def calctd(t, rh, **kwargs):
    """
    Calculate dewpoint from temperature and relative humidity.

    Parameters
    ----------
    t : array_like, 2D
        Temperatures in Kelvin.
    rh : array_like, 2D
        Relative humidities (0. - 100.).
    ni : int, optional
        Number of rows to calculate dewpoint for, default is all rows.

    Returns
    -------
    td : array, 2D
        Dewpoints in Kelvin. Will have same shape as t.

    Notes
    -----
    No quality control is peformed in this routine.

    Examples
    --------
    >>> import aoslib
    >>> aoslib.calctd([[300.]], [[50.]])
    array([[ 288.70455933]], dtype=float32)

    """
    return _awips.calctd(t, rh, **kwargs)

For example, t and rh will be recast to (1, N) 2D array if they are 1D:

In [38]: a = np.ones((12), dtype='float64') * 300.

In [39]: b = np.arange(12.)In [42]: c = aoslib.calctd(a, b)

In [42]: c = aoslib.calctd(a, b)

In [43]: c.shapeIn 

Out[43]: (12, 1)

If either one is a 3D array it will raise an error, although the traceback is not all that informative:

In [66]: b.shape = (12,)

In [67]: a.shape = (2, 3, 2)

In [68]: c = aoslib.calctd(a, b,)
0-th dimension must be fixed to 2 but got 12
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
/home/jhelmus/python/aoslib/aoslib/<ipython-input-68-89e0de225c80> in <module>()
----> 1 c = aoslib.calctd(a, b,)

/home/jhelmus/python/aoslib/aoslib/awips.pyc in calctd(t, rh, **kwargs)
     35 
     36     """
---> 37     return _awips.calctd(t, rh, **kwargs)

error: failed in converting 2nd argument `rh' of _awips.calctd to C/Fortran array

In [69]: a.shape = (12, )

In [70]: b.shape = (2, 3, 2)

In [71]: c = aoslib.calctd(a, b,)
too many axes: 3 (effrank=3), expected rank=2
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
/home/jhelmus/python/aoslib/aoslib/<ipython-input-71-89e0de225c80> in <module>()
----> 1 c = aoslib.calctd(a, b,)

/home/jhelmus/python/aoslib/aoslib/awips.pyc in calctd(t, rh, **kwargs)
     35 
     36     """
---> 37     return _awips.calctd(t, rh, **kwargs)

error: failed in converting 2nd argument `rh' of _awips.calctd to C/Fortran array

And if the shapes of t and rh do not match again we get an error

 In [47]: a.shape = (3, 4)

In [48]: b.shape = (4, 3)

In [49]: c = aoslib.calctd(a, b)
0-th dimension must be fixed to 3 but got 4
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
/home/jhelmus/python/aoslib/aoslib/<ipython-input-49-6c0643141292> in <module>()
----> 1 c = aoslib.calctd(a, b)

/home/jhelmus/python/aoslib/aoslib/awips.pyc in calctd(t, rh, **kwargs)
     35 
     36     """
---> 37     return _awips.calctd(t, rh, **kwargs)

error: failed in converting 2nd argument `rh' of _awips.calctd to C/Fortran array

And if ni is larger than the number of rows:

In [57]: aoslib.calctd(a, b, ni=4)
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
/home/jhelmus/python/aoslib/aoslib/<ipython-input-57-30a88ce323df> in <module>()
----> 1 aoslib.calctd(a, b, ni=4)

/home/jhelmus/python/aoslib/aoslib/awips.pyc in calctd(t, rh, **kwargs)
     35 
     36     """
---> 37     return _awips.calctd(t, rh, **kwargs)

error: (ni<=shape(t,0)) failed for 1st keyword ni: calctd:ni=4

The error messages are a bit cryptic, but I would say these checks are as good a assert statements in the code. Longer term we could add more robust parameter checking which raise ValueError or other appropriate Errors but I think at first the check provided by f2py are likely sufficient.

jwblin commented 11 years ago

Thanks, Jonathan H.! I've been mulling over the idea of using f2py signatures to do some error checking and kind of going back-and-forth on it (for the reasons I mentioned earlier). I think I agree that taking the extra step of putting together nice signature files is worth the effort, even if the first wrapping of the AWIPS functions is just a first step in developing the package. What do others think? I'm particularly interested in the thoughts of those who are planning on helping with the first wrap. Dave? Others?

jjhelmus commented 11 years ago

I think that good f2py signatures give us an acceptable level of error checking, especially if we plan on possibly replacing the FORTRAN code at a later date.

david-ian-brown commented 11 years ago

Hi Jonathan, I am trying to understand what you mean by "adjusting the f2py signature". Did you modify the signature for the calctd2 routine from what was automatically generated by f2py? If so, how did you change it? Many of the signatures in the wrap file have similar directives such as "optional, check(shape(…))", etc. Sorry if I'm being obtuse. -dave

On May 8, 2013, at 1:59 PM, "Jonathan J. Helmus" notifications@github.com wrote:

I think that good f2py signatures give us an acceptable level of error checking, especially if we plan on possibly replacing the FORTRAN code at a later date.

— Reply to this email directly or view it on GitHub.

jjhelmus commented 11 years ago

dave

The example should have have calctd, not calctd2, I updated the signature in my post above.

Check out the changes to the signature file in commit 176bcf2b70488c40a65362403af8c45e57f851fe .

You are correct that most of the checks are already included in the automatically generated signature, the only one I added was the check on ni. The more important step in adjusting the f2py signature is adding the intent(in), intent(hide), intent(out) statements. Without these the wrapped function behaves like a FORTRAN function in Python, that is to say you would pass all the input and output variables as parameters and the output would be updated in place, example f(a, b, c, out) with no return value. With the intent statements the function behaves much like we would expect from a Python function, example out = f(a, b, c).

I preferred to do this is by adding directive as comment in the FORTRAN file, but given that we want to leave the FORTRAN files as untouched as possible, adjusting the signature file also works.

david-ian-brown commented 11 years ago

OK, great! Thanks for the explanation. Also now that I understand it does seem to me like the way to proceed. -dave

On May 9, 2013, at 4:43 AM, "Jonathan J. Helmus" notifications@github.com wrote:

dave

The example should have have calctd, not calctd2, I updated the signature in my post above.

Check out the changes to the signature file in commit 176bcf2 .

You are correct that most of the checks are already included in the automatically generated signature, the only one I added was the check on ni. The more important step in adjusting the f2py signature is adding the intent(in), intent(hide), intent(out) statements. Without these the wrapped function behaves like a FORTRAN function in Python, that is to say you would pass all the input and output variables as parameters and the output would be updated in place ( example f(a, b, c, out) with no return value ). With the intent statements the function behaves much like we would expect from a Python function (example out = f(a, b, c).

I preferred to do this is by adding directive as comment in the FORTRAN file, but given that we want to leave the FORTRAN files as untouched as possible, adjusting the signature file also works.

— Reply to this email directly or view it on GitHub.