Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gsw_ct_freezing and gsw_t_freezing switched to "exact" #10

Merged
merged 1 commit into from
Apr 3, 2017

Conversation

efiring
Copy link
Member

@efiring efiring commented Mar 31, 2017

This is the minimal change; if backwards compatibility is
not required, we could rename the "exact" versions instead.

This partly addresses #9; see also TEOS-10/GSW-Fortran#4.

This is the minimal change; if backwards compatibility is
not required, we could rename the "exact" versions instead.
@efiring
Copy link
Member Author

efiring commented Mar 31, 2017

@richardsc, @dankelley would you look at this, please? From your perspective, is there a need to retain the redundant "exact" function names? Are R users using them?

@dankelley
Copy link
Contributor

We are at the early stages with the R package. I don't think there are too many people using it, beyond as a way to get CT and SA. The decision we made was to have functions named similarly to those of Matlab, as a way to help users to make a transition. So, for example, the R function gsw_CT_freezing() calls a C function named gsw_ct_freezing_exact ... and I'd prefer it if the C source were updated, because someone looking at our code might wonder what's going on.

I think what you're saying, Eric, is that you are going to make the same choice in your python work. I've not checked the details of other functions. Basically, I'm quite neutral on what things are called in the C code, because I can do a git diff to see if things were renamed.

To be honest, I'm a bit out of touch on who is in charge of which sets of code. I'd be in favour of a half-hour web-video meeting with all involved (hard, with the timezones), to get an idea for distributing labour amongst participants. I'd also like to get an idea on the versioning scheme; the teos-10 webpages made me think that the C was for 3.05, but the code is partly 3.03, as far as I can tell. The reason the versioning matters is that I'd like to name the R code as e.g. 3.05-1 for a first draft, 3.05-2 for a second, etc ... but if the original (matlab) changes, I'd jump to 3.06-1 etc. I want users to know (a) the version of the base algorithms and (b) the version of my wrapper.

@efiring
Copy link
Member Author

efiring commented Mar 31, 2017

@dankelley,
Thanks for the response. It sounds like the summary answer to my immediate question is that my PR looks OK as far as R usage is concerned, and that extending it to drop the two "exact" names would also be OK.

Versioning and a conference: yes, I agree 100%. The versioning has been murky and incomplete, and it would be good to have some better communication among people interested in any or all of the TEOS language implementations.

I'm not sure whether a video conference is feasible, spanning eastern Canada to Australia, but we need to do something.

@hylandg hylandg merged commit 57192cb into TEOS-10:master Apr 3, 2017
@efiring
Copy link
Member Author

efiring commented Apr 3, 2017

@hylandg Thanks for the merge. I decided to go ahead with a thorough cleanup of this part of the API, so I will have a PR for that momentarily.

@PaulMBarker
Copy link
Member

The version number. (first_number.seond_number.third_number)

The first number indicated which version of the look up table is used to make salinity, there have been 3 version to date. I doubt we will see a 4th version for a long time.

The second number relates to significant changes to the calculated values such as specific volume etc. or if we introduce a whole new section.

The third number is for small bug fixes.

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.

None yet

4 participants