Skip to content

implement sa_from_sp_baltic#95

Draft
DocOtak wants to merge 2 commits into
castelao:mainfrom
DocOtak:sa_sp_baltic
Draft

implement sa_from_sp_baltic#95
DocOtak wants to merge 2 commits into
castelao:mainfrom
DocOtak:sa_sp_baltic

Conversation

@DocOtak

@DocOtak DocOtak commented Feb 13, 2026

Copy link
Copy Markdown

This is my first attempt ever at contributing to a rust project so, it's probably going to be rough. I also realize there are no doc strings yet and this potentially conflicts with #92 but wanted to "dip my toe in" to use the idiom.

Some questions/thoughts I had while doing this in no real order:

  • How much is the rust version a "C" port vs a implementation in idiomatic rust? E.g. it feels like I would model the Baltic coordinate data with some sort of Point{lon, lat} struct rather than the 2 sets of 2 arrays.
  • Similarly, the in_baltic function is not in the C version, but is duplicated in both sa_from_sp_baltic and sp_from_sa_baltic
  • for future, sa_from_sp_baltic (and sp_from_sa_baltic) do not appear to be public interface for gsw, the higher level sp_from_sa and sa_from_spcall the *_baltic functions internally, check to see if they return anything valid and if not, do the SAAR lookup/conversion.
  • Using the const generic in util_xinterp1, forces a compile time check that the two array sizes are the same which is nice, the util_indx cannot enforce a "2 or more" constraint until generic_const_exprs is stable.
  • Is match allowed? I don't think I saw it used in other locations.

@castelao

Copy link
Copy Markdown
Owner

Hi @DocOtak ! Thanks for your contribution!

Good questions. No, it is not just translate to Rust. We shall keep the signature of the public functions. And we shall provide the same results, but if make sense we can use features to decide the behavior. For instance, the compat must reproduce exactly the MatLab implementation. The internals is on us. For instance in this case it would probably make more sense to use match since it is quite a small set of numbers. In a glance, it looks like GSW-C reproduced the GSW-Matlab, which has find and interp1d for free (or already paid the cost of it).

I agree on how you used pub. Everything is local, but sa_from_sp_baltic is expected to be used within the crate. I don't remember if it is used all around or only in this module by sa_from_sp() only? If that is the case, it won't even need the pub(crate).

Since those two functions require will require some support local functions plus all the related tests, I was wondering about moving those to a sub-module and then export the public one to mod.rs. What do you think about about that?

Yes, we could use a custom type Point. It might be overkill to be used just here, but we benefit from the zero cost abstraction, so if you want to go this route for the practice, I support you. I think that the official manual covers a custom Point type right? If you go that route, I think it would be important to isolate all this in a submodule and bring to mod.rs only the public inerface.

Thank you!!

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.

2 participants