Add test to top-level Makefile to ensure PNetCDF is working #1263
Conversation
Makefile
Outdated
| &int main(){\n\ | ||
| & MPI_Info info;\n\ | ||
| & int err=0, cmode=NC_NOCLOBBER, ncid;\n\ | ||
| & err = ncmpi_create(MPI_COMM_WORLD, \"foo.nc\", cmode, info, &ncid);\n\ |
There was a problem hiding this comment.
Although this test program only needs to verify that we can compile, and runtime correctness is immaterial, it still seems like it would be nice to have code here that wouldn't generate compile-time warnings. info is used uninitialized, and can simply be replaced in the call to ncmpi_create with MPI_INFO_NULL. There's also no need to initialize err to zero, since it's clearly set in the program; and, initializing err but not ncid is a bit inconsistent. Also, in my opinion it's simpler to just use NC_NOCLOBBER inline in the call to ncmpi_create rather than having to declare another variable (cmode).
Makefile
Outdated
| @echo "*********************************************************"; \ | ||
| echo "ERROR: The PNETCDF environment variable isn't set."; \ | ||
| echo "Please set this variable to the location PNetCDF is installed"; \ | ||
| echo "for the system you are using"; \ |
There was a problem hiding this comment.
I'd argue that "for the system you are using" should go without saying; and so if we can get the error across with fewer words, that might be preferable.
There was a problem hiding this comment.
Are you fine with removing the "for the system you are using" text?
There was a problem hiding this comment.
Thanks for the reminder. I just adjusted that with fixup commit cbef541.
Makefile
Outdated
| else | ||
| @echo "*********************************************************"; \ | ||
| echo "ERROR: The PNETCDF environment variable isn't set."; \ | ||
| echo "Please set this variable to the location PNetCDF is installed"; \ |
There was a problem hiding this comment.
It's a minor nitpick, but the documentation for the PnetCDF library seems to not capitalize the N.
gdicker1
force-pushed
the
framework/makefile_pnetcdf_test
branch
from
558f843 to
cb7a885
Compare
|
@mgduda the most 2 recent fixup commits should resolve the PNETCDF variable unset message and the include directives. |
|
@gdicker1 This looks good to me! Feel free to squash the commit history into a single commit. |
Adds a build target (pnetcdf_test) that attempts to build a test PnetCDF C program. Failing to build the test program causes a warning message to the user, leaves the test program and build log in the top-level directory, and aborts the MPAS build. The MPAS build will also abort if the pnetcdf_test detects the PNETCDF environment variable isn't set.
gdicker1
force-pushed
the
framework/makefile_pnetcdf_test
branch
from
cbef541 to
31e791d
Compare
|
@mgduda, done! |
With the addition of SMIOL, PNetCDF is required for all builds of MPAS. This PR prevents build issues that can arise from forgetting the PNETCDF environment variable or other issues related to finding the PNetCDF library. If the build fails, the PNetCDF test C program will be left in the MPAS-Model directory to help users troubleshoot.