NOAA-EMC / NCEPLIBS-ip2

Other
1 stars 4 forks source link

Does this library have functions the same name as the ip library? #18

Closed edwardhartnett closed 2 years ago

edwardhartnett commented 3 years ago

@GeorgeGayno-NOAA was the approach taken that the same function names are used in ip and ip2?

edwardhartnett commented 3 years ago

@GeorgeGayno-NOAA and @kgerheiser can we discuss this issue please?

Is this the issue that makes it difficult to build both the ip and ip2 library?

kgerheiser commented 3 years ago

They do, and with slightly different interfaces.

But It's not even the names, really. The code commonality is copy-paste similar. The interfaces need to be split out (grib1 and grib2) and an intermediate layer calls the common interpolation code.

edwardhartnett commented 3 years ago

@GeorgeGayno-NOAA would be happy for your input.

Seems like a good incremental plan would be:

GeorgeGayno-NOAA commented 3 years ago

@GeorgeGayno-NOAA would be happy for your input.

Seems like a good incremental plan would be:

  • Add "2" to the end of all the function names in ip2.
  • Copy the ip2 code to an ip2 subdirectory of ip; retire the ip2 repo.
  • Use fortran interfaces if you want to have multiple functions share the same name. Then the API must be different.
  • Eliminate the duplication of code gradually.

@edwardhartnett That is one way to proceed. But I wonder if it would be less overall work to go directly to the final solution.

kgerheiser commented 3 years ago

I agree with George. I think it's best to just do it in one go. I don't think it's worthwhile to do it incrementally like that. It's not that difficult.

edwardhartnett commented 3 years ago

What is the reason for trying to reuse the function names? So that the codes that use ip can be more readily upgraded to grib2?

Let us say what you propose was complete - would we not have broken all the code that uses the ip functions, wherever we changed the API?

Will the old codes that currently use ip benefit from these new functions that can handle both grib1 and grib2? Seems like they were using grib1, they will continue to use grib1.

kgerheiser commented 3 years ago

You don't have to break the API and if you do there's only a couple of routines.

There are two main public facing routines (for each library): ipolates (scalar) and ipolatev (vector).

I propose making an interface for them, keeping the function signature the same. Then, those top-level routines can handle their specific grid definitions (grib1 or grib2) and they pass their data to the chosen interpolation routine, and the interpolation routines will be re-factored so they're not specific to grib1 or grib2 and can be used interchangeably between ip and ip2.

edwardhartnett commented 3 years ago

I have to take a look at the code before discussing further. ;-) I will take a look. If you want to whip up a PR demonstrating this, that would be helpful.

Incremental is a better approach usually. The goal is not to minimize programmer keystrokes. ;-)

kgerheiser commented 3 years ago

I think I have a good solution coming up

edwardhartnett commented 3 years ago

OK, a working code example is worth a thousand plans and proposals!

edwardhartnett commented 3 years ago

See #23

edwardhartnett commented 2 years ago

I believe this issue has been solved, and I will close it...