[VCMML-487] Use setup.py with pkg-config (#117)

The build system of the python and fortran models are somewhat coupled. This PR addresses a few sources of this coupling

- Remove the hardcoded names in the `setup.py` leveraging the `pkg-config` tool and #54
- `lib/Makefile` no longer depends on any makefiles or source code in `lib/external`. It uses pkg-config as well.

Some other changes:
- Remove `build_docker.sh` script in lieu of a makefile target.
- Streamline the Dockerfile by basing the final wrapper image of the fv3gfs-wrapper-build image. This was needed for the build to pass, and reduces the redundant configuration of esmf and fms.

Work not done:
- Substantial restructuring of the relationship between `docker/Dockerfile` and `lib/external/docker/Dockerfile`. This should be addressed in future PRs.

.circleci/config.yml

@@ -91,7 +91,7 @@ jobs:
           name: "Compile model and perform model tests"
           command: |
             echo "$ENCODED_GCR_KEY" | base64 --decode | docker login --username _json_key --password-stdin https://us.gcr.io
-            DOCKER_BUILDKIT=1 BUILDKIT_PROGRESS=plain BUILD_FROM_INTERMEDIATE=y bash test_docker.sh
+            DOCKER_BUILDKIT=1 BUILDKIT_PROGRESS=plain BUILD_FROM_INTERMEDIATE=y make test-docker
       - save_cache:
           paths:
             - /home/circleci/.cache/fv3gfs/fv3config-cache

Makefile

@@ -106,9 +106,9 @@ docs-docker:
 	$(BROWSER) docs/_build/html/index.html
 
 build-docker:
-	./build_docker.sh
+	BUILD_FROM_INTERMEDIATE=y $(MAKE) -C docker
 
-test-docker:
+test-docker: build-docker
 	./test_docker.sh
 
 servedocs: docs ## compile the docs watching for changes

README.md

@@ -19,8 +19,8 @@ This package uses submodules. After you check out the repository, you must run
 Local Machine Installation
 --------------------------
 
-The Docker image can be built using `build_docker.sh`, or built and then
-tested using `test_docker.sh` (which will use the existing build if present).
+The Docker image can be built using `make build-docker`, or built and then
+tested using `make test-docker` (which will use the existing build if present).
 The first time you build, both ESMF and FMS will be
 built, taking up quite a lot of time. On subsequent builds, these may be retrieved
 from cached images, if you allow caching on your system.
@@ -66,7 +66,7 @@ docker image with bind-mounts into your local filesystem. Just be sure to `make
 when you're done to remove the build artifacts, or it may cause problems when you
 build the docker image.
 
-With the image already built by `build_docker.sh` or pulled using
+With the image already built by `make build-docker` or pulled using
 `docker pull us.gcr.io/vcm-ml/fv3gfs-wrapper`, run `dev_docker.sh`. This will
 bind-mount the `fv3gfs`, `lib`, `tests`, `external`, and `templates` directories into the
 docker image. Inside the docker image, you can build or re-build the model with

docker/Dockerfile

@@ -62,23 +62,18 @@ COPY --from=fv3gfs-esmf /usr/local/esmf/lib/libO3/Linux.gfortran.64.mpiuni.defau
 COPY lib /fv3gfs-wrapper/lib
 COPY --from=fv3gfs-fortran-build ${FV3GFS_BUILD_ROOT}/stochastic_physics /fv3gfs-wrapper/lib/external/stochastic_physics
 COPY --from=fv3gfs-fortran-build ${FV3GFS_BUILD_ROOT}/FV3 /fv3gfs-wrapper/lib/external/FV3
+
+RUN PREFIX=/usr/local make -C /fv3gfs-wrapper/lib/external/FV3 install
+
 COPY templates /fv3gfs-wrapper/templates
 COPY examples /fv3gfs-wrapper/examples
 COPY docs /fv3gfs-wrapper/docs
 # files can be copied in one line
 COPY fv3gfs/wrapper/dynamics_properties.json fv3gfs/wrapper/physics_properties.json /fv3gfs-wrapper/fv3gfs/wrapper/
 COPY fill_templates.py HISTORY.md LICENSE Makefile MANIFEST.in README.md setup.cfg setup.py /fv3gfs-wrapper/
-
-# make sources first, because -j8 doesn't propagate through setup.py layer
-RUN cd /fv3gfs-wrapper/lib && \
-    make -j8
-
 COPY fv3gfs /fv3gfs-wrapper/fv3gfs
 
-# compile wrapper
-RUN cd /fv3gfs-wrapper && \
-    make build
-
+RUN make -C /fv3gfs-wrapper build
 
 #-----------------------------------------------------------------------------
 FROM $FORTRAN_ENV_IMAGE AS fv3gfs-dawn-build
@@ -115,44 +110,31 @@ RUN git clone --depth 1 -b fv3_validation https://github.com/eddie-c-davis/gt4py
 FROM $DAWN_IMAGE AS fv3gfs-dawn
 
 #-----------------------------------------------------------------------------
-FROM fv3gfs-wrapper-env AS fv3gfs-wrapper
-
-ENV FMS_DIR=/FMS
-ENV ESMF_DIR=/esmf
-ENV ESMF_INC="-I${ESMF_DIR}/include -I${ESMF_DIR}/mod/modO3/Linux.gfortran.64.mpiuni.default/"
-
-ENV FMS_LIB=${FMS_DIR}/libFMS/.libs/
-ENV ESMF_LIB=${ESMF_DIR}/lib/libO3/Linux.gfortran.64.mpiuni.default/
-ENV LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:${ESMF_LIB}:${FMS_LIB}
+FROM fv3gfs-wrapper-build AS fv3gfs-wrapper
 
-ENV FV3CONFIG_CACHE_DIR=/inputdata
-
-COPY --from=fv3gfs-fms /FMS $FMS_DIR
-COPY --from=fv3gfs-esmf /usr/local/esmf ${ESMF_DIR}
-COPY --from=fv3gfs-fms /FMS/libFMS/.libs/*.so* /lib64/
-COPY --from=fv3gfs-esmf /usr/local/esmf/lib/libO3/Linux.gfortran.64.mpiuni.default/*.so* /lib64/
+# Install Dawn and gt4py
 COPY --from=fv3gfs-dawn /usr/src/dawn /usr/src/dawn
 COPY --from=fv3gfs-dawn /dawn4py-0.0.2-cp37-cp37m-linux_x86_64.whl /usr/src/dawn/dawn4py-0.0.2-cp37-cp37m-linux_x86_64.whl
 COPY --from=fv3gfs-dawn /usr/src/gt4py /usr/src/gt4py
-
 RUN pip3 install --no-deps --no-cache-dir /usr/src/dawn/dawn4py-0.0.2-cp37-cp37m-linux_x86_64.whl
 RUN pip3 install --no-deps --no-cache-dir /usr/src/gt4py
 
-COPY --from=fv3gfs-wrapper-build /fv3gfs-wrapper /fv3gfs-wrapper
-COPY tests /fv3gfs-wrapper/tests
-
 # copy and install dependency packages
 COPY external /fv3gfs-wrapper/external
 COPY requirements_local.txt /fv3gfs-wrapper/requirements_local.txt
 RUN cd /fv3gfs-wrapper && pip3 install --no-deps --no-cache-dir -r /fv3gfs-wrapper/requirements_local.txt
 
 # cache model data
+ENV FV3CONFIG_CACHE_DIR=/inputdata
 COPY --from=fv3config-inputdata /fv3config-cache $FV3CONFIG_CACHE_DIR/fv3config-cache
 
 RUN chmod -R 777 $FV3CONFIG_CACHE_DIR && \
     echo "ulimit -s unlimited" >> /etc/bash.bashrc && \
     mkdir /outdir && \
     chmod -R 777 /outdir
 
+# copy the tests
+COPY tests /fv3gfs-wrapper/tests
+
 # interactive shell by default
 CMD ["bash"]

lib/Makefile

@@ -1,48 +1,33 @@
 SHELL = /bin/sh
 
-ifndef FV3GFS_BUILD_DIR
-	FV3GFS_BUILD_DIR=$(shell pwd)/external/FV3
-endif
-include $(FV3GFS_BUILD_DIR)/conf/configure.fv3
-
 TEMPLATES = $(shell ls ../templates)
 PROPERTIES_FILES = ../fv3gfs/wrapper/dynamics_properties.json ../fv3gfs/wrapper/physics_properties.json
 
-FFLAGS += \
-	-I$(FMS_DIR) \
-	-I$(FMS_DIR)/include \
-	-I$(FV3GFS_BUILD_DIR)/gfsphysics \
-	-I$(FV3GFS_BUILD_DIR)/ipd \
-	-I$(FV3GFS_BUILD_DIR)/cpl \
-	-I$(FV3GFS_BUILD_DIR)/io \
-	-I$(FV3GFS_BUILD_DIR)/atmos_cubed_sphere \
-	-I$(FV3GFS_BUILD_DIR)/ccpp/driver \
-	-I$(FV3GFS_BUILD_DIR)/../stochastic_physics \
-	-I$(FV3GFS_BUILD_DIR)
+# Use pkg-config to get flags for fv3
+# If we run without PKG_CONFIG_ALLOW_SYSTEM_CFLAGS,
+# the -I/usr/include flag will be missing
+FFLAGS += $(shell PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config fv3 --cflags)
+FC ?= mpif90
 
 .PHONY: clean cleanall fortran_libs
 
 all: coupler_lib.o dynamics_data.o physics_data.o $(TEMPLATES)
 
-dynamics_data.o: dynamics_data.F90 fortran_libs
-	$(FC) $(CPPDEFS) $(CPPFLAGS) $(FPPFLAGS) $(FFLAGS) $(OTHERFLAGS) $(OTHER_FFLAGS) $(ESMF_INC) -c $< -o $@
+dynamics_data.o: dynamics_data.F90
+	$(FC) $(FFLAGS)  -c $< -o $@
 
-physics_data.o: physics_data.F90 dynamics_data.o fortran_libs
-	$(FC) $(CPPDEFS) $(CPPFLAGS) $(FPPFLAGS) $(FFLAGS) $(OTHERFLAGS) $(OTHER_FFLAGS) $(ESMF_INC) -c $< -o $@
+physics_data.o: physics_data.F90 dynamics_data.o
+	$(FC) $(FFLAGS)  -c $< -o $@
 
-coupler_lib.o: coupler_lib.F90 fortran_libs
-	$(FC) $(CPPDEFS) $(CPPFLAGS) $(FPPFLAGS) $(FFLAGS) $(OTHERFLAGS) $(OTHER_FFLAGS) $(ESMF_INC) -c $< -o $@
+coupler_lib.o: coupler_lib.F90
+	$(FC) $(FFLAGS)  -c $< -o $@
 
 clean:
 	@echo "Cleaning ... "
 	$(RM) -f *.o *.mod *.lst *.c depend $(TEMPLATES)
 	$(RM) -rf tests/pytest/output/*
-	(cd external && make clean)
 
 $(TEMPLATES): %: ../templates/% $(PROPERTIES_FILES)
 	python3 ../fill_templates.py $@
 
-fortran_libs:
-	cd $(FV3GFS_BUILD_DIR) && make
-
 cleanall: clean

pyproject.toml

@@ -0,0 +1,4 @@
+# credit: https://stackoverflow.com/questions/60045913/installing-numpy-before-using-numpy-distutils-core-setup
+[build-system]
+build-backend = "setuptools.build_meta"
+requires=["numpy", "setuptools", "cython", "pkgconfig", "wheel"]

requirements.txt

@@ -83,3 +83,4 @@ websocket-client==0.57.0
 xarray==0.16.0
 zarr==2.3.2
 zipp==0.5.2
+pkgconfig==1.5.0

setup.py

@@ -1,11 +1,14 @@
 import os
-import shutil
-import sys
 from setuptools import setup, find_namespace_packages
 from distutils.extension import Extension
+
+# Specify these build requirements in pyproject.toml
+# https://www.python.org/dev/peps/pep-0518/
 from Cython.Distutils import build_ext
 from Cython.Build import cythonize
 
+import pkgconfig
+
 # This line only needed if building with NumPy in Cython file.
 from numpy import get_include
 
@@ -26,55 +29,36 @@ class BuildDirectoryError(Exception):
     "lib/dynamics_data.o",
 ]
 
-relative_fv3gfs_build_filenames = [
-    "atmos_model.o",
-    "module_fv3_config.o",
-    "cpl/libfv3cpl.a",
-    "ipd/libipd.a",
-    "atmos_cubed_sphere/libfv3core.a",
-    "coarse_graining/libfv3coarse_graining.a",
-    "io/libfv3io.a",
-    "gfsphysics/libgfsphys.a",
-    "../stochastic_physics/libstochastic_physics.a",
-    "/opt/NCEPlibs/lib/libnemsio_d.a",
-    "/opt/NCEPlibs/lib/libbacio_4.a",
-    "/opt/NCEPlibs/lib/libsp_v2.0.2_d.a",
-    "/opt/NCEPlibs/lib/libw3emc_d.a",
-    "/opt/NCEPlibs/lib/libw3nco_d.a",
-]
+wrapper_build_filenames = []
+for relative_filename in relative_wrapper_build_filenames:
+    wrapper_build_filenames.append(os.path.join(package_dir, relative_filename))
+
+# order of library link args matters
+# dependencies must be to the right of dependees
+# https://stackoverflow.com/questions/45135/why-does-the-order-in-which-libraries-are-linked-sometimes-cause-errors-in-gcc
+library_link_args = []
+library_link_args.extend(wrapper_build_filenames)
+library_link_args += pkgconfig.libs("fv3").split()
 
 mpi_flavor = os.environ.get("MPI", "openmpi")
 if mpi_flavor == "openmpi":
-    mpi_fortran_lib = "-lmpi_mpifh"
+    library_link_args += pkgconfig.libs("ompi-fort").split()
 else:
-    mpi_fortran_lib = "-lmpifort"
-
-library_link_args = [
-    "-lFMS",
-    "-lesmf",
-    "-lgfortran",
-    "-lpython3." + str(sys.version_info.minor) + "m",
-    mpi_fortran_lib,
-    "-lmpi",
-    "-lnetcdf",
-    "-lnetcdff",
-    "-fopenmp",
-    "-lmvec",
-    "-lblas",
-    "-lc",
-    "-lrt",
-]
+    library_link_args += pkgconfig.libs("mpich-fort").split()
+
+# need to include math and c library
+library_link_args += ["-lmvec", "-lc"]
 
 requirements = [
+    "mpi4py>=3",
     "cftime>=1.2.1",
     "xarray>=0.15.1",
     "netCDF4>=1.4.2",
-    "numpy",
+    "numpy>=1.16",
+    "pyyaml>=5",
     f"fv3gfs-util>=0.5.1",
 ]
 
-setup_requirements = ["cython", "numpy", "jinja2"]
-
 test_requirements = []
 
 with open("README.md") as readme_file:
@@ -88,35 +72,15 @@ class BuildDirectoryError(Exception):
 else:
     fv3gfs_build_path = os.path.join(package_dir, "lib/external/FV3/")
 
-fortran_build_filenames = []
-for relative_filename in relative_fv3gfs_build_filenames:
-    fortran_build_filenames.append(os.path.join(fv3gfs_build_path, relative_filename))
-
-wrapper_build_filenames = []
-for relative_filename in relative_wrapper_build_filenames:
-    wrapper_build_filenames.append(os.path.join(package_dir, relative_filename))
-
-for filename in fortran_build_filenames:
-    if not os.path.isfile(filename):
-        raise BuildDirectoryError(
-            f"File {filename} is missing, first run make in {fv3gfs_build_path}"
-        )
-
-# copy2 preserves executable flag
-shutil.copy2(
-    os.path.join(fv3gfs_build_path, "fv3.exe"), os.path.join(package_dir, "fv3.exe")
-)
 
 ext_modules = [
     Extension(  # module name:
         "fv3gfs.wrapper._wrapper",
         # source file:
         ["lib/_wrapper.pyx"],
         include_dirs=[get_include()],
-        extra_link_args=wrapper_build_filenames
-        + fortran_build_filenames
-        + library_link_args,
-        depends=fortran_build_filenames + wrapper_build_filenames,
+        extra_link_args=library_link_args,
+        depends=wrapper_build_filenames,
     )
 ]
 
@@ -135,7 +99,6 @@ class BuildDirectoryError(Exception):
         "Programming Language :: Python :: 3.7",
     ],
     install_requires=requirements,
-    setup_requires=setup_requirements,
     tests_require=test_requirements,
     name="fv3gfs-wrapper",
     license="BSD license",

test_docker.sh

@@ -6,8 +6,6 @@ if [ -z "$DOCKER_IMAGE" ]; then
     DOCKER_IMAGE=us.gcr.io/vcm-ml/fv3gfs-wrapper
 fi
 
-./build_docker.sh
-
 pytest ./tests/image_tests/*.py
 
 if [[ "GOOGLE_APPLICATION_CREDENTIALS" == "" ]]
@@ -17,4 +15,4 @@ else
     docker run -v $GOOGLE_APPLICATION_CREDENTIALS:$GOOGLE_APPLICATION_CREDENTIALS \
         --env GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS \
         -it $DOCKER_IMAGE bash -c "cd /fv3gfs-wrapper; make test"
-fi
+fi