Skip to content

Comments

Adding Gibbs#132

Closed
pvthinker wants to merge 4 commits intoTEOS-10:mainfrom
pvthinker:gibbs
Closed

Adding Gibbs#132
pvthinker wants to merge 4 commits intoTEOS-10:mainfrom
pvthinker:gibbs

Conversation

@pvthinker
Copy link

@pvthinker pvthinker commented Apr 6, 2023

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
Copy link
Member

efiring commented Apr 6, 2023

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
Copy link
Author

pvthinker commented Apr 7, 2023

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
Copy link
Contributor

DocOtak commented Apr 8, 2023

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

@efiring
Copy link
Member

efiring commented Apr 8, 2023

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
Copy link
Member

efiring commented Jun 21, 2023

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.

@efiring efiring closed this Jun 21, 2023
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.

add missing functions (gibbs etc)

3 participants