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
143 stars 30 forks source link

add gibbs signature #130

Closed ocefpaf closed 1 year ago

ocefpaf commented 1 year ago

Working #128

pvthinker commented 1 year ago

Thanks ocefpaf for acting so fast. I'm amazed.

ocefpaf commented 1 year ago

@efiring I'm struggling with the logic in https://github.com/TEOS-10/GSW-Python/blob/30c5aa4c623d5ce1cf5f190844054833b79b5cda/tools/c_header_parser.py#L86-L92

That prevents the addition of this function b/c it has int arguments.

PS: we are missing gsw_gibbs_ice too for the same reason.

ocefpaf commented 1 year ago

See https://github.com/TEOS-10/GSW-C/pull/59 for the first step.

efiring commented 1 year ago

I think that what is needed for gibbs is a custom function in src/method_bodies.c with a corresponding entry in src/method_def_entries.c. Like geo_strf_dyn_height and others there, the underlying C function has a signature that doesn't fit any of the generic patterns that can be handled by the auto-generation machinery, so a little hand-made C is required.

efiring commented 1 year ago

To elaborate: the iiiddd_d signatures of gibbs and gibbs_ice actually are compatible with ufuncs, and could even be handled via changes to the signature parsing and the ufunc generation code, but adding that support probably is not trivial. One-off hand-written ufuncs in method_bodies might be easier. It's not entirely clear.

pvthinker commented 1 year ago

Thanks for your efforts. I think one of the issue is that the triplet of integers cannot be treated like the triplet of double.

When one apply gibbs to np.ndarray of (sa,ct,p) one wants to do it for a single triplet of integer (ns, nt, np)

btw : note the potential name conflict between 'np' the third argument of gibbs and 'np' the usual nickname for numpy

On my side, I've come up with a patch that works but it's completely not aligned with the package policy/style/guidelines.

gsw_gibbs is clearly the core of teos-10 and we would like to have it in 'gsw-python' but beyond that one, there are other secondary functions that could be included. Namely all the ones having this integer parameter, e.g. gsw_nsquared to give one. If a general patch to handle those int is found then I believe they all come up easily into the package.

Cheers,

ocefpaf commented 1 year ago

but adding that support probably is not trivial

Yep. I tried yesterday to implement that and ended up with a code that was too convoluted and I gave up.