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 core_physics dependencies for mpas_atmphys_lsm_noahmpfinalize module #1204

Merged

Conversation

islas
Copy link
Contributor

@islas islas commented Jul 3, 2024

Module mpas_atmphys_lsm_noahmpfinalize added in #1161 does not have the appropriate dependencies in the respective makefile.

This adds the necessary dependencies to the mpas_atmphys_lsm_noahmpfinalize.o

@islas islas force-pushed the mpas_atmphys_lsm_noahmpfinalize-deps branch from 526f4b0 to df7bce8 Compare July 9, 2024 18:24
@mgduda mgduda self-requested a review July 16, 2024 18:48
@mgduda
Copy link
Contributor

mgduda commented Jul 17, 2024

@islas Can you add some detail to the commit message (and use capitalization where appropriate)? A couple of questions to consider for the commit message:

  • what defines "minimum dependencies"?
  • how have we determined that "mpas_atmphys_vars.o" is the minimum dependency?

@islas
Copy link
Contributor Author

islas commented Jul 17, 2024

Is this roughly what you are looking for from the commit message?

Add the minimum object dependencies in the makefile for mpas_atmphys_lsm_noahmpfinalize.o. This corresponds to all unresolved modules used yet to be compiled by the time this rule is evaluated.
Modules originating from NoahMP and framework are ignored as they should be handled separately before the core_physics target is started.

I'm refraining from saying "NoahMP and framework are already compiled by the time the core_physics target is started" since the fix is based on a commit that does not have those fix in it, thus would be referring to future patches that have yet to occur if one were to try and take this individual commit.

@islas islas force-pushed the mpas_atmphys_lsm_noahmpfinalize-deps branch from df7bce8 to f940356 Compare July 19, 2024 21:17
…ize.o

Add the object dependencies in the makefile for
mpas_atmphys_lsm_noahmpfinalize.o specific to the objects created under
core_physics, i.e. any use statement that reference modules compiled as part of
the core_physics target.

Modules originating from NoahMP and framework are ignored as they should be
handled separately before the core_physics target is started.
@islas islas force-pushed the mpas_atmphys_lsm_noahmpfinalize-deps branch from f940356 to 5f25019 Compare July 19, 2024 22:22
@islas islas changed the title Add minimum dependencies for lsm noahmp finalize module Add core_physics dependencies for mpas_atmphys_lsm_noahmpfinalize module Jul 19, 2024
@mgduda mgduda merged commit 0a8dd86 into MPAS-Dev:hotfix-v8.2.1 Aug 6, 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