User/f1p/merge gex ddep #1637

fabienpaulot · 2025-01-27T14:49:38Z

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:

My code follows the style guidelines of this project
[x ] I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules
New check tests, if applicable, are included
make distcheck passes

Examples: "atm_to_lnd_ex","coupler_mod","wetbc" units = kg/(m2 s) / "lnd_to_atm_ex","coupler_mod","test1" units = kg/m2/year /

rem1776 · 2025-01-27T21:26:35Z

@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:

FMS/CMakeLists.txt

Lines 114 to 116 in 15ec0c7

    
           coupler/atmos_ocean_fluxes.F90 
        
           coupler/coupler_types.F90 
        
           coupler/ensemble_manager.F90

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.

fabienpaulot · 2025-01-27T21:45:42Z

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: ***@***.***>

rem1776 · 2025-01-27T22:00:28Z

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 _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:

FMS/CMakeLists.txt

Lines 114 to 116 in 15ec0c7

coupler/atmos_ocean_fluxes.F90

coupler/coupler_types.F90

coupler/ensemble_manager.F90

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 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.

…m and CMAkeLists.txt

fabienpaulot · 2025-01-27T22:37:06Z

@rem1776 I think I fixed the space/line length/CMake but it's still failing one check

rem1776 · 2025-02-03T15:07:38Z

@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

Needs to have tracer_manager before coupler in the SUBDIRS list so the module dependency is satisfied.

…so the module dependency is satisfied per Ryan's recommendation

fabienpaulot · 2025-02-03T15:57:11Z

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: ***@***.***>

thomas-robinson

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.

thomas-robinson · 2025-02-03T15:42:56Z

coupler/gex.F90

@@ -0,0 +1,220 @@
+!>simple routine to pass non-tracer fields across components (regrid)
+
+module gex_mod


The module should be named fms_coupler_gex_mod because it is a new module in FMS in the coupler subdirectory.

thomas-robinson · 2025-02-03T15:46:52Z

coupler/gex.F90

+
+implicit none ; private
+
+public :: gex_init, gex_get_index,gex_get_n_ex, gex_get_property, gex_name, gex_units


All of these public routines should be prefixed with fms_coupler_

public :: fms_coupler_gex_init, fms_coupler_gex_get_index, fms_coupler_gex_get_n_ex, fms_coupler_gex_get_property, fms_coupler_gex_name, fms_coupler_gex_units

thomas-robinson · 2025-02-03T15:48:53Z

coupler/gex.F90

@@ -0,0 +1,220 @@
+!>simple routine to pass non-tracer fields across components (regrid)


The doxygen formatting should be updated to reflect that this is a module. Please refer to other modules in FMS to see a proper example.

Also, please add more detail to the documentation. What is gex an abbreviation for?

thomas-robinson · 2025-02-03T15:50:50Z

coupler/gex.F90

+character(3) :: module_name = 'gex'
+logical      :: initialized = .FALSE.
+
+integer, parameter :: gex_name  = 1
+integer, parameter :: gex_units = 2
+
+type gex_type
+   character(fm_field_name_len):: name  = ''
+   character(fm_string_len)    :: units = ''
+   logical                     :: set   = .FALSE.
+end type gex_type
+type gex_type_r
+   type(gex_type), allocatable:: field(:)
+end type gex_type_r
+
+integer,          allocatable :: n_gex(:,:)
+type(gex_type_r), allocatable :: gex_fields(:,:)


Every variable needs to be documented with the correct doxygen format.

thomas-robinson · 2025-02-03T15:51:38Z

coupler/gex.F90

+   if (mpp_pe()==mpp_root_pe()) then
+      write(*,*) ''
+      write(*,*) '####################################'
+      write(*,*) ''
+   end if


Why does this happen?

my attempt to do pretty formatting. Feel free to edit/remove @thomas-robinson

thomas-robinson · 2025-02-03T15:55:17Z

coupler/gex.F90

+   character(len=*), intent(in) :: listroot  ! name of the field manager list
+   integer,          intent(in) :: MODEL_SRC ! index of the model where the tracer comes FROM
+   integer,          intent(in) :: MODEL_REC ! index of the model where the tracer goes TO
+
+   type(fm_list_iter_type)      :: iter ! iterator over the list of tracers
+   character(fm_field_name_len) :: name = '' ! name of the tracer
+   character(fm_type_name_len)  :: ftype ! type of the field table entry (not used)
+!    character(fm_path_name_len)  :: current_list ! storage for current location in the fiels manager tree
+   character(fm_path_name_len)  :: listname  ! name of the field manager list for each tracer
+
+   integer :: n


These variables all need doxygen style comments

thomas-robinson · 2025-02-03T15:56:40Z

coupler/gex.F90

+   integer, intent(in)                         :: MODEL_SRC, MODEL_REC
+   integer gex_get_n_ex


document each variable with doxygen

thomas-robinson · 2025-02-03T15:57:01Z

coupler/gex.F90

+   integer, intent(in)   :: MODEL_SRC, MODEL_REC,index
+   integer               :: property
+   character(len=64)     :: gex_get_property


please document each variable with doxygen style comments

thomas-robinson · 2025-02-03T15:57:21Z

coupler/gex.F90

+   integer, intent(in)                         :: MODEL_SRC, MODEL_REC
+   logical, intent(in), optional               :: record    !record that this exchanged has been found and will be set
+
+   integer :: i
+   integer :: gex_get_index


please document each variable with doxygen style comments

thomas-robinson · 2025-02-03T16:06:07Z

coupler/gex.F90

@@ -0,0 +1,220 @@
+!>simple routine to pass non-tracer fields across components (regrid)


The license header is missing.

fabienpaulot and others added 7 commits August 2, 2024 10:55

increase number of exchanged fileds

86ed4da

formatting

2021155

changes for gex

bc241ea

Add ability to read units for tracers in GEX sections of field table

1579834

Examples: "atm_to_lnd_ex","coupler_mod","wetbc" units = kg/(m2 s) / "lnd_to_atm_ex","coupler_mod","test1" units = kg/m2/year /

Merge branch 'user/f1p/increase_xgrid_max_fields' into user/f1p/gex

1af1b38

change spacing

7d97893

increase max number of exchanged tracers for dry deposition

9833739

fabienpaulot requested review from uramirez8707, bensonr and rem1776 as code owners January 27, 2025 14:49

rem1776 mentioned this pull request Jan 27, 2025

User/f1p/merge gex ddep NOAA-GFDL/FMScoupler#151

Open

fabienpaulot added 2 commits January 27, 2025 17:21

Renamed generic_exchange to gex to match FMS style. Update Makefile.a…

b7a26bb

…m and CMAkeLists.txt

Remove whitespace

30dbf61

move Needs to have tracer_manager before coupler in the SUBDIRS list …

2bdd1cb

…so the module dependency is satisfied per Ryan's recommendation

thomas-robinson requested changes Feb 3, 2025

View reviewed changes

fabienpaulot added 2 commits February 3, 2025 12:59

Formatting/Doc

745f2c7

White space

5536f7e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User/f1p/merge gex ddep #1637

User/f1p/merge gex ddep #1637

fabienpaulot commented Jan 27, 2025 •

edited

Loading

rem1776 commented Jan 27, 2025

fabienpaulot commented Jan 27, 2025 via email

rem1776 commented Jan 27, 2025

fabienpaulot commented Jan 27, 2025

rem1776 commented Feb 3, 2025

fabienpaulot commented Feb 3, 2025 via email

thomas-robinson left a comment

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

fabienpaulot Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

thomas-robinson Feb 3, 2025

		@@ -0,0 +1,220 @@
		!>simple routine to pass non-tracer fields across components (regrid)

		module gex_mod


		implicit none ; private

		public :: gex_init, gex_get_index,gex_get_n_ex, gex_get_property, gex_name, gex_units

		integer, intent(in) :: MODEL_SRC, MODEL_REC
		integer gex_get_n_ex

User/f1p/merge gex ddep #1637

Are you sure you want to change the base?

User/f1p/merge gex ddep #1637

Conversation

fabienpaulot commented Jan 27, 2025 • edited Loading

rem1776 commented Jan 27, 2025

fabienpaulot commented Jan 27, 2025 via email

rem1776 commented Jan 27, 2025

fabienpaulot commented Jan 27, 2025

rem1776 commented Feb 3, 2025

fabienpaulot commented Feb 3, 2025 via email

thomas-robinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabienpaulot commented Jan 27, 2025 •

edited

Loading