search

TEOS-10 / GSW-Python

Python implementation of TEOS-10 GSW based on ufunc wrappers of GSW-C
https://teos-10.github.io/GSW-Python
Other
140 stars 30 forks source link

Adding Gibbs #132

Closed pvthinker closed 1 year ago

pvthinker commented 1 year ago

This PR resolves #131

Algo: use ctypes to import the compiled function that is present in the dynamic library [suffix .so on Linux]

then call the function with the float passed into ctypes.c_double()

efiring commented 1 year ago

This approach abandons the advantages of a ufunc, and drops back to using a Python loop to call the compiled function. Broadcasting could be added in the Python function, but the Python-level looping would remain. I would much rather take a little time to explore alternatives that keep the gibbs implementations closer to the bulk of the library. The ctypes method can be used outside the library in the interim.

pvthinker commented 1 year ago

We can make gibbs a ufunc with the help of numpy.frompyfunc.

This works fine but the __doc__ is lost by frompyfunc, which forces the default numpy.ufunc __doc__. I didn't manage to circumvent this. It's a known issue. The __doc__ attribute of an object returned by frompyfunc is unwritable.

Speaking of ufunc, it seems like none of the gsw functions are of numpy.ufun type. They are simply function. In particular, they don't have the out keyword, that is useful when the result variable has already been allocated because it avoids an unnecessary allocation.

DocOtak commented 1 year ago

@efiring why isn't the C gsw_gibbs function already being wrapped by the scripts in tools?

efiring commented 1 year ago

I simply did not include signatures with integer arguments; it did not seem necessary, and it adds a level of complexity relative to restricting the machinery to handling the normal user-level functions. I didn't think anyone would want to call the gibbs function directly; I viewed it as an internal engine, not something that needed to be exposed. Other than purely utility functions, gibbs and gibbs ice are the only functions in C that are suitable for ufuncs and are not currently handled. I'm looking into adding them.

efiring commented 1 year ago

Thank you for the contribution. Although I did not merge it, it prompted me to make some needed clarifications and revisions in the process of coming up with the alternative approach I used in #134.