Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@islas
Copy link
Contributor

@islas islas commented Jul 2, 2024

The original additions of checking lib64/ for pnetcdf/netcdf (#592) made assumptions that when the variable supplied for the respective library location, if a lib64/ folder exists to use that. This leads to problems when existence of the actual library is not checked and if a lib64/ folder does exist but the library is not installed there. As this configuration layout is not unusual, the fix is to keep the precedence of lib64/, but check that the library exists within the directory before assigning values.

This follows the same logic as before with the added check that a libpnetcdf.*
file exists in the checked directory. The change in logic will now account for
situations where PNETCDF is set to a directory containing both lib/ and lib64/
directories and libpnetcdf.* exists in just the lib/ folder. Should libpnetcdf.*
exist in both directories, the lib64/ still maintains precedence.
@islas
Copy link
Contributor Author

islas commented Jul 2, 2024

Future work should consider using pkg-config or any of the pnetcdf-config / nc-config / nf-config to supply the appropriate link / compile flags. This would generally reduce the maintenance burden from rewriting logic that is already provided by those projects. Likewise, this would lean towards more standard package / dependency management rather than project-specific variables.

@mgduda mgduda changed the base branch from master to hotfix-v8.2.1 July 2, 2024 20:02
@mgduda mgduda self-requested a review July 5, 2024 23:11
@mgduda
Copy link
Contributor

mgduda commented Jul 17, 2024

@islas The commit message for ef3c4b3 references a commit (3dab782) that doesn't seem to (no longer?) exists. Could you update the commit message?

@islas
Copy link
Contributor Author

islas commented Jul 17, 2024

Ah! I cleaned up with a rebase before updating the remote, but didn't update the appropriate descriptions. Incoming fix

As the netcdf library logic mirrors the pnetcdf one, it would suffer
from the same issues noted in 0ca4208 about lib/lib64 checking.
This updates the corresponding issue in the netcdf library detection
before it becomes an issue.
@islas islas force-pushed the check-pnetcdf-exists branch from ef3c4b3 to fe46d4f Compare July 17, 2024 01:45
@islas
Copy link
Contributor Author

islas commented Jul 17, 2024

@mgduda mgduda merged commit c314013 into MPAS-Dev:hotfix-v8.2.1 Jul 19, 2024
mgduda added a commit that referenced this pull request Aug 7, 2024
This merge addresses several issues in the MPAS-Atmosphere model and in the MPAS
infrastructure. Specific changes in this merge include:

* Improved detection of an 'mpi_f08' module (PR #1202), as well as improved
  detection of netCDF and PnetCDF library paths (PR #1203), in the top-level
  Makefile.

* The addition of a missing dependency in the physics Makefile to correct
  parallel build issues (PR #1204).

* Fixes to the CMake build files used by MPAS-JEDI (PR #1205).

* Fixes to double-precision builds of MPAS-Atmosphere (PR #1207, PR #1208).

* Correction of the calculation of height AGL used in the computation of 1-km
  radar reflectivity fields (PR #1213).

* Correction of an issue that prevented the MYNN PBL scheme from being used
  without also using the Thompson aerosol-aware microphysics (PR #1215).

* Fixes to allow MPAS-Atmosphere to be built without physics (i.e., dynamics-
  only builds) (PR #1221).

* Various code cleanup and minor corrections (PR #1206, PR #1212, PR #1224,
  PR #1226).
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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.