User/f1p/merge gex ddep #1637
User/f1p/merge gex ddep #1637
Conversation
Examples: "atm_to_lnd_ex","coupler_mod","wetbc" units = kg/(m2 s) / "lnd_to_atm_ex","coupler_mod","test1" units = kg/m2/year /
|
@fabienpaulot Thanks for submitting these changes. I think we might need to change the module name to After that this will need some more changes to get the build systems working properly since a new module is being added. For cmake, it would just need the new Lines 114 to 116 in 15ec0c7 For autotools, you'll need to edit Once you made the module name change. Let me know if need help/have any questions, hope this wasn't too confusing. |
|
Can I change generic_exchange.F90 to gex.F90 instead? This would make my
life much easier. Fabien
…On Mon, Jan 27, 2025 at 4:27 PM Ryan Mulhall ***@***.***> wrote:
@fabienpaulot <https://github.com/fabienpaulot> Thanks for submitting
these changes. I think we might need to change the module name to
generic_exchange_mod. For FMS we typically follow the <filename>_mod
naming scheme for any modules.
After that this will need some more changes to get the build systems
working properly since a new module is being added.
For cmake, it would just need the new coupler/generic_exchange.F90 file
path added to CMakeLists.txt, along with the other files from that
directory:
https://github.com/NOAA-GFDL/FMS/blob/15ec0c735cba2780d3da37f4e1e3b5d4a969b4ff/CMakeLists.txt#L114-L116
For autotools, you'll need to edit coupler/Makefile.am. The
libcoupler_la_SOURCES will need the new file name added as well, and then
MODFILES will need the compiled module output (ie. module <name> in the
file), which would be:
generic_exchange_mod.$(FC_MODEXT) \
Once you made the module name change.
Let me know if need help/have any questions, hope this wasn't too
confusing.
—
Reply to this email directly, view it on GitHub
<#1637 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKAQA45VKGSIVHLY6V3OHTT2M2QCDAVCNFSM6AAAAABV6IKIECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJWHEZDSOJVGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Yeah that works too! As long as it follows the convention it should be fine. |
|
@rem1776 I think I fixed the space/line length/CMake but it's still failing one check |
Looks like its just failing the autotools build from a file dependency. I think to fix it, you would have to make this change in the top level Needs to have |
…so the module dependency is satisfied per Ryan's recommendation
|
Ok I tried to make the changes that you requested (I am not sure I
completely understand....). Let me know if this works. Fabien
…On Mon, Feb 3, 2025 at 10:08 AM Ryan Mulhall ***@***.***> wrote:
@rem1776 <https://github.com/rem1776> I think I fixed the space/line
length/CMake but it's still failing one check
Looks like its just failing the autotools build from a file dependency. I
think to fix it, you would have to make this change in the top level
Makefile.am:
main...rem1776:FMS:pr1637
#diff-0462e381b2fb3286568215681c8983490a37ac9ae0f0c5ee304df7fa6426d4af
<main...rem1776:FMS:pr1637#diff-0462e381b2fb3286568215681c8983490a37ac9ae0f0c5ee304df7fa6426d4af>
Needs to have tracer_manager before coupler in the SUBDIRS list so the
module dependency is satisfied.
—
Reply to this email directly, view it on GitHub
<#1637 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKAQA43WS6DCPCFMW7W3C532N6A5BAVCNFSM6AAAAABV6IKIECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZRGI3DSMBUGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
- Please make sure that the doxygen style is followed for the module, module variables, subroutines, and subroutine variables. Please make sure documentation style is consistent with other modules in this repository (ie. remove the
!######s that are around routine documentation.) An example can be found here https://github.com/NOAA-GFDL/FMS/blob/main/field_manager/fm_yaml.F90 - New modules and routines should be prefixed with
fms_coupler_gex_. This is consistent with the modernized approach in FMS. The filename should be fms_coupler_gex.F90. - The public routines need to be added to libFMS.F90 and made public through that interface. That's how they should be accessed in the FMScoupler also (through
use FMS) - The new file needs the LGPL licence header.
* Add unit tests for gex_mod * Refactor gex.F90 to conform to FMS code style * Add boundary checking of input indices to public routines in gex_mod
|
@J-Lentz test seems to be failing. Could you please take a look. Thanks |
If FMS is built without YAML support, then the gex unit test based on a YAML field table should be skipped.
|
I've pushed the necessary changes to my
|
|
@jesse Lentz - NOAA Affiliate ***@***.***>
Do you mind fixing this minor issue since this involves a short summary of
what test_coupler_gex does. Thanks.
…On Wed, Feb 26, 2025 at 1:29 PM uramirez8707 ***@***.***> wrote:
***@***.**** requested changes on this pull request.
This looks good to me. Just one *final* thing :D
Thanks for the documentation!
------------------------------
In test_fms/coupler/test_gex.F90
<#1637 (comment)>:
> @@ -0,0 +1,131 @@
+program test_coupler_gex
This test needs the license header
—
Reply to this email directly, view it on GitHub
<#1637 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKAQA4Y5XMS2VAI4VB4S77T2RYBZFAVCNFSM6AAAAABV6IKIECVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNBVGQ3DSMJZGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Description
Update to add
a) ability to pass deposition fluxes from the atmosphere to the land (gex)
b) sink of gas/aerosol on land handled by LM4 [increase number of exchanged tracer]
Fixes # (issue)
n/a
How Has This Been Tested?
Tested on gaea (ESM4p5)
Checklist:
make distcheckpasses