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

Add option for internally compiling GRIB2 libs (zlib, libpng, and JasPer) #183

Merged
merged 9 commits into from
Nov 5, 2021

Conversation

mgduda
Copy link
Collaborator

@mgduda mgduda commented Oct 28, 2021

This PR adds a new command-line option to the configure script, --build-grib2-libs,
that causes copies of the zlib, libpng, and JasPer libraries to be compiled and installed
in the WPS/grib2 directory. When --build-grib2-libs is provided as an argument to
the configure script, the environment variables JASPERLIB and JASPERINC are
ignored, and the compiled ungrib and g2print executables will use the internally
built GRIB2 libraries.

Source code for the following library versions is now included in the WPS/external directory:

  • zlib 1.2.11
  • libpng 1.6.37
  • JasPer 1.900.29

The JasPer library version is not the latest available at the time of this PR, but it was chosen
because it is apparently the last version to support configuration with the GNU autoconf
tools rather than requiring CMake.

@mgduda
Copy link
Collaborator Author

mgduda commented Oct 28, 2021

I've verified that I can successfully compile ungrib with the internally compiled GRIB2 libraries with the GNU, PGI, Intel, and LLVM compilers on Linux, and with the GNU compilers on macOS.

@smileMchen
Copy link
Collaborator

I just git clone this PR codes and compile in cheyenne. All the executable files can be generated successfully. A test case is done using geogrid.exe, ungrib.exe and metgrid.exe produced here.

2 similar comments
@smileMchen
Copy link
Collaborator

I just git clone this PR codes and compile in cheyenne. All the executable files can be generated successfully. A test case is done using geogrid.exe, ungrib.exe and metgrid.exe produced here.

@smileMchen
Copy link
Collaborator

I just git clone this PR codes and compile in cheyenne. All the executable files can be generated successfully. A test case is done using geogrid.exe, ungrib.exe and metgrid.exe produced here.

@smileMchen
Copy link
Collaborator

@mgduda
Michael,
This PR works fine. I just notice that In total 1046 files have been changed, --- I suppose most of these changes are related to those newly added external libraries?

@kkeene44
Copy link
Collaborator

@mgduda
I also successfully built it on Cheyenne, using the --build-grib2-libs configure option.

@mgduda
Copy link
Collaborator Author

mgduda commented Oct 28, 2021

@mgduda Michael, This PR works fine. I just notice that In total 1046 files have been changed, --- I suppose most of these changes are related to those newly added external libraries?

That's right -- most of the 1046 changed files are the source files for the zlib, libpng, and JasPer libraries that have been added to the repository by this PR.

@smileMchen smileMchen closed this Oct 28, 2021
@mgduda mgduda reopened this Oct 28, 2021
@mgduda
Copy link
Collaborator Author

mgduda commented Oct 28, 2021

It just occurred to me that it would be better to just commit the JasPer 1.900.29 library version in the first place rather than first committing JasPer 1.900.1 and later updating. I'll rework the commit history so that the JasPer 1.900.29 library is committed immediately, and I'll verify that the end result is an identical branch.

…' directory

The specific library versions included in this commit are:
 - zlib 1.2.11
 - libpng 1.6.37
 - JasPer 1.900.29
…macOS

The gcc 9.2.0 compiler stops with the following errors when compiling the
jpg_dummy.c file on macOS:

  jpg_dummy.c:90:14: error: conflicting types for 'jpg_decode'
     90 | jas_image_t *jpg_decode(jas_stream_t *in, char *optstr)
        |              ^~~~~~~~~~
  In file included from jpg_dummy.c:70:
  ../../../src/libjasper/include/jasper/jas_image.h:518:14: note: previous declaration of 'jpg_decode' was here
    518 | jas_image_t *jpg_decode(jas_stream_t *in, const char *optstr);
        |              ^~~~~~~~~~
  jpg_dummy.c:104:5: error: conflicting types for 'jpg_encode'
    104 | int jpg_encode(jas_image_t *image, jas_stream_t *out, char *optstr)
        |     ^~~~~~~~~~
  In file included from jpg_dummy.c:70:
  ../../../src/libjasper/include/jasper/jas_image.h:519:5: note: previous declaration of 'jpg_encode' was here
    519 | int jpg_encode(jas_image_t *image, jas_stream_t *out, const char *optstr);
        |     ^~~~~~~~~~

This commit modifies the argument lists in the jpg_decode and jpg_encode
functions so that they match the declarations of these functions in jas_image.h.
When invoking 'make' in the external directory, the following variables
must be defined:

   CC - the C compiler with which libraries should be built
   INTERNAL_GRIB2_PATH - the path where libraries will be installed
…B2 libs

Since the files created during the build of GRIB2 libraries may vary under
different compilers, it may be necessary to update the list of files after
testing with a broader set of compilers.
The GRIB2 libraries should only be compiled if GRIB2 support was requested.
…ompiled

This commit addresses several issues with the dependencies in the
ungrib/src/Makefile file that prevented just the g2print utility from being
compiled:

- parse_table.o was missing a dependency on module_stringutil.o
- g2print.exe was missing a dependency on lib$(LIBTARGET).a
- Since lib$(LIBTARGET).a depends on $(OBJS2) and $(OBJS2) includes
  parse_table.o, $(OBJS2) should also include module_stringutil.o
The 'grib2' directory is where any internally compiled GRIB2 libraries will be
installed if the --build-grib2-libs option is given to the configure script.
@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@mgduda I also successfully built it on Cheyenne, using the --build-grib2-libs configure option.

@kkeene44 If you've finished with your testing, could you leave an approving review? No worries at all if there's still testing you'd like to do -- I just wasn't sure.

@kkeene44
Copy link
Collaborator

kkeene44 commented Nov 2, 2021

@mgduda
I apologize for forgetting to approve it! You should be good to go now.

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@kkeene44 It just occurred to me that if you do happen to have time this week, it might be good to know if the changes in this PR work in one of the tutorial AMIs.

@davegill Could you also test this PR in one of your containers?

@kkeene44
Copy link
Collaborator

kkeene44 commented Nov 2, 2021

@mgduda
Okay, I'll give it a try in an AWS instance and keep you posted.

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
With the build_internal_grib2_libs branch

Linux GNU 7.3.0 (classroom machine)
Standard build: builds ungrib.exe and g2print.exe
With configure --build-grib2-libs: builds OK, specifically -> ungrib.exe and g2print.exe

Linux GNU 9.3.1 (docker container)
Standard build: builds ungrib.exe and g2print.exe
With configure --build-grib2-libs: builds OK, specifically -> ungrib.exe and g2print.exe

For the --build-grib2-libs option, the WPS/grib2 dir was populated.

I zapped the built code in the ungrib subdirectory. On re-build (just compile) the grib2 directory was identified as "ready to go".

I then zapped the ungrib subdirectory again, and then removed all of the grib2 directory. On a re-build, the grib2 lib was built straightaway.

Then I tried to be sneaky, and only got rid of the g2print.exe and g2print.o. I zapped the grib2 directory, and it all built as it should.

I was unable to break it with a few reasonable possible user scenarios. I generated output with ungrib and looked at output with g2print. This seems solid.

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Why so many file changes? Did you go back in the commit history so everyone could cherry pick this commit?

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@kkeene44
This PR is to the develop branch, so maybe it sees the light of day in Apr 2022.
In a coordinated fashion, I think that https://www2.mmm.ucar.edu/wrf/OnLineTutorial/compilation_tutorial.php should be updated to use the more simple technique.

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Michael,
When I use "configure --build-grib2-libs", it seems like the compiler options could be cut in half. Is that slated for when this capability becomes an eventual default?

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@mgduda Why so many file changes? Did you go back in the commit history so everyone could cherry pick this commit?

We are adding the source code for three different libraries.

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@mgduda Michael, When I use "configure --build-grib2-libs", it seems like the compiler options could be cut in half. Is that slated for when this capability becomes an eventual default?

We could certainly do that, but I'd suggest a separate PR to keep this one focused on adding the capability to build internal copies of zlib, libpng, and JasPer.

@mgduda mgduda merged commit 9f62674 into wrf-model:develop Nov 5, 2021
@mgduda mgduda deleted the build_internal_grib2_libs branch November 5, 2021 17:24
@kkeene44
Copy link
Collaborator

kkeene44 commented Nov 9, 2021

@kkeene44 It just occurred to me that if you do happen to have time this week, it might be good to know if the changes in this PR work in one of the tutorial AMIs.

@mgduda
I tested this in an AWS cloud instance and it works just fine!

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 9, 2021

@kkeene44 It just occurred to me that if you do happen to have time this week, it might be good to know if the changes in this PR work in one of the tutorial AMIs.

@mgduda I tested this in an AWS cloud instance and it works just fine!

Thanks for testing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants