search

NOAA-EMC / wgrib2

Provides functionality for interacting with, reading, writing, and manipulating GRIB2 files.
27 stars 13 forks source link

Add support for USE_IPOLATES and USE_SPECTRAL #3

Closed kgerheiser closed 4 years ago

kgerheiser commented 4 years ago

Should be easy, I just hope Wesley doesn't use a modified version of these libraries.

kgerheiser commented 4 years ago

The ip lib packaged with wgrib2 is modified from our NCEPLIBS-ip.

I will build what's there, but in the future wgrib2 should try to use the mainline NCEPLIBS-ip and bring in any functionality required.

aerorahul commented 4 years ago

Can we look at what the difference in the ip library is and why that should be used? Ping Wesley.

climbfuji commented 4 years ago

Can we look at what the difference in the ip library is and why that should be used? Ping Wesley.

Let's try to do this right from the beginning. Don't build any dependencies underneath wgrib2 - just the wgrib2 source code itself.

kgerheiser commented 4 years ago

Includes a config.h which defines USE_POLATES (and wherever that IFDEF is used)

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/ipolatev.F90#L1

Added arguments to the polates subroutines

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/ipolatev.F90#L2

IGDT etc

Also in the polates routines themselves

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/polates0.f90#L1

Check grid is new:

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/polates0.f90#L317

aerorahul commented 4 years ago

can this be worked with ip library code-managers? i.e. add these codes in ip repository? I understand the need to get this in quickly, but I think this should be done properly from the beginning.

kgerheiser commented 4 years ago

@GeorgeGayno-NOAA do you know anything about this? It says you wrote some of this code.

GeorgeGayno-NOAA commented 4 years ago

@GeorgeGayno-NOAA do you know anything about this? It says you wrote some of this code.

I am code manager for IP2. Let me see what Wesley changed.

kgerheiser commented 4 years ago

The extra arguments seem to be to call Check_Grids# before each call to the actual ipolates subroutine, which hasn't changed that I can see.

GeorgeGayno-NOAA commented 4 years ago

@kgerheiser I can open a IP2 branch, add Wesley's changes, then compile it for you. What machine are you working on?

kgerheiser commented 4 years ago

I can compile and get it working just fine.

The point is that wgrib2 shouldn't rely on a forked version of ip lib, and that we should get wgrib2 to work with the standard ip lib either by bringing wgrib2's changes into ip or by modifying wgrib2 (or both), if that's a viable path forward.

kgerheiser commented 4 years ago

Do you have any thoughts on the actual changes themselves? Like the CHECK_GRIDS routine or other things you may have seen?

GeorgeGayno-NOAA commented 4 years ago

Includes a config.h which defines USE_POLATES (and wherever that IFDEF is used)

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/ipolatev.F90#L1

Yes, this 'config.h' file is new and will need to be added.

Added arguments to the polates subroutines

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/ipolatev.F90#L2

IGDT etc

I don't see any new arguments in ipolatesv or the polates routines (I am comparing Wesley's local copy to https://github.com/NOAA-EMC/NCEPLIBS-ip2/commit/3eb7f4baa21e1b482dd06ec9e456a6c7a558c412)

Also in the polates routines themselves

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/polates0.f90#L1

Check grid is new:

https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/blob/ba3d196fbd566d4ba09fb57a563b9245e265618e/grib2/ip2lib_d/polates0.f90#L317

kgerheiser commented 4 years ago

Oh, is ip2 different than just ip? Why are there two?

GeorgeGayno-NOAA commented 4 years ago

Oh, is ip2 different than just ip?

Now I understand the confusion. Yes, ip2 and ip are different libraries. The former is based on the grib2 standard, the latter on the grib1 standard. According to Boi's email, he sets IPOLATES=3, which is the grib2 version (ip2). So let's try to get the ip2 option to work first. I am not we even need to bother with the ip option as ip2 is based on the more accurate grib2 grid definition templates.

kgerheiser commented 4 years ago

That changes a lot. Wesley responded to me and said that the ip2 that he uses is basically stock from whenever he took it. I guess the CHECK_GRID thing was just removed since then?

I'm going to add a CMake build to ip2 based off of ip's build, then I'll try building wgrib2 with it.

GeorgeGayno-NOAA commented 4 years ago

That changes a lot. Wesley responded to me and said that the ip2 that he uses is basically stock from whenever he took it. I guess the CHECK_GRID thing was just removed since then?

I'm going to add a CMake build to ip2 based off of ip's build, then I'll try building wgrib2 with it.

OK. I will add/test Wesley's changes in this branch: https://github.com/GeorgeGayno-NOAA/NCEPLIBS-ip2/tree/feature/wgrib2

kgerheiser commented 4 years ago

I'm hoping this will work without any changes (besides maybe the #ifdef), but we can get rid of the Config.h and just pass a compile definition through CMake to define it.

GeorgeGayno-NOAA commented 4 years ago

I'm hoping this will work without any changes (besides maybe the #ifdef), but we can get rid of the Config.h and just pass a compile definition through CMake to define it.

Yes, a compile definition through CMake makes sense. For the official ip2 build, USE_SPECTRAL must be '1'. That means wgrib2 will need to link to splib.

kgerheiser commented 4 years ago

I don't know that you need any changes. I was able to build wgrib2 using the stock ip2lib.

Like USE_SPECTRAL, I say we just require sp rather than ifdefing it

Do you need to change SPLAT to IP_SPLAT? Can't we just link to SPLAT in SP?

And that looks like everything.

webisu commented 4 years ago

If I remember correctly, SPLAT in ip2d is different from sp. Caused malfunctioning code. (Sometimes make produced good code, sometimes it produced bad code with minor changes to the makefile.)

Spectral is turned off because I don't know where the spectral library came from. I don't want to distribute wgrib2 with the spectral library until I know the library is open source.

kgerheiser commented 4 years ago

This is the SP I'm using

https://github.com/NOAA-EMC/NCEPLIBS-sp

GeorgeGayno-NOAA commented 4 years ago

If I remember correctly, SPLAT in ip2d is different from sp. Caused malfunctioning code. (Sometimes make produced good code, sometimes it produced bad code with minor changes to the makefile.)

Spectral is turned off because I don't know where the spectral library came from. I don't want to distribute wgrib2 with the spectral library until I know the library is open source.

ip2 and ip call splat the same way. There should be no difference.

aerorahul commented 4 years ago

so ip and ip2 are the same libraries, with the exception of how they handle (or not) grib2. Why are there 2 libraries then and why can they not be unified into a single one?

GeorgeGayno-NOAA commented 4 years ago

I don't know that you need any changes. I was able to build wgrib2 using the stock ip2lib.

Like USE_SPECTRAL, I say we just require sp rather than ifdefing it

Do you need to change SPLAT to IP_SPLAT? Can't we just link to SPLAT in SP?

And that looks like everything.

Then maybe we are done. The only change I see that we should include is a more precise lat/lon calculation for rotated lat/lon 'e' stagger grids: https://github.com/GeorgeGayno-NOAA/NCEPLIBS-ip2/commit/3968f3b915563148238d28c17eef0d49d3b576b9#diff-3ce8c7a97869c5c2adb4be5ae6aacae4

webisu commented 4 years ago

George,

| ip2 and ip call splat the same way. There should be no difference.

The source code splat in ip2lib is different from sp_v2.0.2. Yes, you could make the codes identical. However, you really shouldn't have two codes with the same name. Some linker will complain.

Combining ip & sp is a valid idea. However, ip should be a double precision library for precision issues. Some programs only need the sp routines, and those programs would want to run the sp routines in single precision.

Wesley

On Thu, Jun 25, 2020 at 9:32 AM GeorgeGayno-NOAA notifications@github.com wrote:

If I remember correctly, SPLAT in ip2d is different from sp. Caused malfunctiong code. (Sometimes make produced good code, sometimes it produced bad code with minor changes to the makefile.)

Spectral is turned off because I don't know where the spectral library came from. I don't want to distribute wgrib2 with the spectral library until I know the library is open source.

ip2 and ip call splat the same way. There should be no difference.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/NCEPLIBS-wgrib2/issues/3#issuecomment-649543400, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIB7ZQOMICVAPQXOW5NB43RYNGXXANCNFSM4OGBUHTA .