Refactor boundary condition interface structure by glemieux · Pull Request #1477 · NGEET/fates

glemieux · 2025-09-30T23:47:12Z

Initial refactor of the FATES interface design

Description:

This pull request refactors the HLM-FATES interface design as an initial step towards enabling "dynamic multi-column" FATES runs for NGEE-Arctic cross cut task 4. In this regard, this redesign is intended to address design assumptions in which all FATES patches on a given site are associated with a single column. Given that this redesign will touch on a large amount of code (on both sides), this PR is intended as a minimum-viable case that introduces the new design setup and only refactors a subset of boundary condition variables to prove the concept.

Given the scope of the undertaking, the refactor is also used as an opportunity to introduce a standard method of passing variable data across the HLM-FATES interface and introducing a common vocabulary for associated variables. The intent is to minimize the amount of necessary shared code between FATES and the HLM.

This pull request must be coordinated with E3SM-Project/E3SM#7482 and ESCOMP/CTSM#3705.

For more details, wee NGEET/fates-users-guide#109 for updates to the developer's guide section detail the interface design document (work in progress)

Collaborators:

@rgknox @ckoven

Expectation of Answer Changes:

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

The in-code documentation has been updated with descriptive comments
The documentation has been assessed to determine if updates are necessary

Integrator

FATES PASS/FAIL regression tests were run
Evaluation of test results for answer changes was performed and results provided
FATES-CLM6 Code Freeze: satellite phenology regression tests are b4b

If satellite phenology regressions are not b4b, please hold merge and notify the FATES development team.

Documentation

Technical Note update:
User's Guide update:

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

…rocedure

Technically we don't really need a subgrid_indices array for the patch-side currently.

…o the api registry type This will help next development steps in which the update of certain variables will be conducted prior to others

…update calls

…ncy for the soil level

Moved to the interfaceparameter module

…ures

These are now handled in Update automatically

…procedure

rgknox · 2026-02-13T16:41:10Z

+    type(fates_interface_variable_type), allocatable :: fates_vars(:) 
+
+    ! Array of keys associated with the interface variables
+    character(len=48), allocatable, private :: key(:)


bump this up in case we want long keys in the future?

rgknox · 2026-02-13T16:42:19Z

+    ! Is registry have a FATES patch that exists?
+    logical, private :: active
+
+    ! Container array of interface variables indexed by key


Do the indices of hlm_vars and fates_vars have correspondence with each other? If so, lets note that here in the description

rgknox · 2026-02-13T16:45:02Z

+    integer :: num_api_vars_update_init_dims      ! number of variables dimensions needed during initialization
+    integer :: num_api_vars_update_init           ! number of variables that update only at initialization
+    integer :: num_api_vars_update_daily          ! number of variables that update daily
+    integer :: num_api_vars_update_timestep       ! number of variables that update on the model timestep


We have variables that need to update at different times during the model time-step. If these transfers need to happen at different times, how can we group them into one "timestep" group?

rgknox · 2026-02-13T16:50:42Z

    currentPatch => currentSite%youngest_patch
    do while(associated(currentPatch))
+
+       if (currentPatch%nocomp_pft_label .ne. nocomp_bareground) then


Was this needed for consistency, speed, removing spurious behavior?

This was for spurious behavior, but might actually be defunct code that should be removed per recent development. I'll double check this.

rgknox · 2026-02-13T16:58:18Z

+         currentPatch => this%sites(s)%oldest_patch
+         do while (associated(currentPatch))
+
+            if (currentPatch%nocomp_pft_label .ne. nocomp_bareground) then


I think one of our plans was to get rid of these conditionals by implementing that refactor that removes bareground patches before this, right? Are we punting on that?

For now. We need this in place until we remove the code that instantiates a fates bareground patch, which is a more significant refactor. That said, we'll need to deal with it during the future multi-column fates pull request.

I'll add a comment above the check noting this.

rgknox · 2026-02-13T17:08:39Z

+   do r = 1, this%npatches
+
+      ! Get the gridcell index
+      g = this%registry(r)%GetGridcellIndex()


I think technically, we match our fates sites with land-units now right? ELM could have multiple topo-units, each with a natural vegetation land-unit that we would want to match with. Matching at the LU level in CLM would also work.

ckoven

Thanks @glemieux, this all looks good to me, only minor comments on first reading. Will continue to think some more about how this all works.

ckoven · 2026-02-18T22:46:04Z

+    subroutine SetLastState(this, last_state)
+
+      ! This procedure sets the last state logical metadata for the calling interface variable


I'm not following (yet) how or why the is_last logic is used in practice. maybe some further details on that would be helpful here.

ckoven · 2026-02-18T23:01:50Z

+      ! Update the litter flux counters not including the total flux counter
+      if (key == hlm_fates_litter_carbon_cellulose .or. &
+          key == hlm_fates_litter_carbon_labile .or. &
+          key == hlm_fates_litter_carbon_lignin .or. &
+          key == hlm_fates_litter_nitrogen_cellulose .or. &
+          key == hlm_fates_litter_nitrogen_labile .or. &
+          key == hlm_fates_litter_nitrogen_lignin .or. &
+          key == hlm_fates_litter_phosphorus_cellulose .or. &
+          key == hlm_fates_litter_phosphorus_labile .or. &
+          key == hlm_fates_litter_phosphorus_lignin) then
+            this%num_api_vars_litter_flux= this%num_api_vars_litter_flux + 1
+      end if


I guess an alternative, which might be easier to maintain, would be to make an 'is_litter_flux' or something like that boolean flag for all interface variables rather than enumerating all the variables here?

ckoven · 2026-02-18T23:03:15Z

+      ! Litter flux update
+      if (this%key(index) == hlm_fates_litter_carbon_cellulose .or. &
+          this%key(index) == hlm_fates_litter_nitrogen_cellulose .or. &
+          this%key(index) == hlm_fates_litter_phosphorus_cellulose .or. &
+          this%key(index) == hlm_fates_litter_carbon_labile .or. &
+          this%key(index) == hlm_fates_litter_nitrogen_labile .or. &
+          this%key(index) == hlm_fates_litter_phosphorus_labile .or. &
+          this%key(index) == hlm_fates_litter_carbon_lignin .or. &
+          this%key(index) == hlm_fates_litter_nitrogen_lignin .or. &
+          this%key(index) == hlm_fates_litter_phosphorus_lignin) then
+            count_litter_flux = count_litter_flux + 1
+            this%filter_litter_flux(count_litter_flux) = index
+      end if


similar comment to the above -- having a boolean flag attached to the variables might be easier to maintain than enumerating them here?

ckoven · 2026-02-18T23:08:38Z

  use shr_infnan_mod      , only : nan => shr_infnan_nan, assignment(=)
+  use PRTGenericMod       , only : prt_cnp_flex_allom_hyp
+  use FatesInterfaceVariableTypeMod, only : fates_interface_variable_type
+  use FatesInterfaceParametersMod


I think I agree that it makes sense to do an unlimited use statement rather than enumerating every possible interface variable name in the list of only variables, which might get quite long, but I guess the risk is that a reader of the code sees all the string indices in DefineInterfaceRegistry and it isn't obvious where those all come from. So I wonder if it would make sense to only have the open-ended use statement in that subroutine and restrict it to use only the integer parameters throughout the rest of the module?

an alternative might be to give the string keys names something that is more obvious what they are, like fates_interface_var_longname?

…ion scalar proc

Refactors testing directory Also makes a minor error message update to the quadratic solver

github-project-automation Bot added this to FATES Pull Request Planning and Status Sep 30, 2025

github-project-automation Bot moved this to Finding Reviewers in FATES Pull Request Planning and Status Sep 30, 2025

glemieux mentioned this pull request Sep 30, 2025

Refactor API boundary condition passing mechanism #1431

Closed

5 tasks

glemieux marked this pull request as draft September 30, 2025 23:48

glemieux removed the status in FATES Pull Request Planning and Status Sep 30, 2025

glemieux added 25 commits October 31, 2025 15:51

add case select logic and pointers to index into the source data pointer

f3c34e7

add type-safe selection constructs to the update interface variable p…

bcfcb30

…rocedure

move subgrid heirarchy values into fates interface types mod

32371cf

remove superseded transfer bc procedures

6e201bc

add intent in to all registry subroutines

1b475e3

remove defunct transferbcin/out procedures in patch type

16924aa

add data size to the var type

c001497

add dimension error diagnostics to the var type

7cf0087

convert variable size comparison error checking to a subroutine

05c8db9

add scalar interface register subroutine

1e6914f

add nlevsoil to patch-level bcin

2e00287

add interface key names as parameter variables

caece8c

move subgrid index types into fatesinterfacetypesmod

bd50484

move patch_api pointer update inside the do while

b4a3edc

todo comment updates

91e6307

removed unused indexers

88d0bd4

Correct which api has the subgrid indice update.

c6b3e97

Technically we don't really need a subgrid_indices array for the patch-side currently.

adding comments for next steps

f5d7486

add update frequency to the registry variable type and and counters t…

e9ada14

…o the api registry type This will help next development steps in which the update of certain variables will be conducted prior to others

add filters to the registry api type to provide shorter loops during …

8b2f8c3

…update calls

update the definteinterface call to pass initialization update freque…

09a11c3

…ncy for the soil level

minor formatting changes

61ef4a3

initialize local variable to avoid implicit save

d3227c8

add registry procedure to update on the variables during initialization

5d34cd5

minor move of use statment

3b0a12d

add interface level subroutine to set last state

ba47570

glemieux marked this pull request as ready for review January 28, 2026 17:36

glemieux moved this to Finding Reviewers in FATES Pull Request Planning and Status Jan 28, 2026

glemieux added 11 commits January 28, 2026 09:46

remove temporary diagnostics from fatessoilbgcfluxmod

e64db77

reinstate area_frac in MIMICS portion of fluxintolitterpools

067e7ee

add comments to the new site type procedures

bb5b4f0

remove unused interface parameter

34b0a1e

add descriptive comments to the new fates interface registry procedures

2b51cf4

remove subgrid level parameters

32c2465

Moved to the interfaceparameter module

add descriptive comments to the new interfacetypemod procedures

29e9a20

add more descriptive comments for the new interfacetype module proced…

106dfff

…ures

add descriptive comments to the new interfacevartype procedures

3a03366

remove unused conversion calls

0a468c9

These are now handled in Update automatically

change conversion (normalization_flag) to be an output of the update …

82137a9

…procedure

glemieux mentioned this pull request Feb 5, 2026

Refactor FATES interface to directly pass to fates patch data structure E3SM-Project/E3SM#7482

Draft

rgknox reviewed Feb 13, 2026

View reviewed changes

ckoven reviewed Feb 18, 2026

View reviewed changes

glemieux mentioned this pull request Feb 19, 2026

Add API design document to the developer's guide NGEET/fates-users-guide#109

Open

glemieux moved this from Finding Reviewers to Under Review in FATES Pull Request Planning and Status Feb 25, 2026

glemieux commented Feb 25, 2026

View reviewed changes

Comment thread main/FatesInterfaceMod.F90

glemieux and others added 5 commits February 25, 2026 14:31

Add comment about active register bareground check

e073d06

remove bareground check by passing the fully bc_array into fragmentat…

8e048e6

…ion scalar proc

Merge tag 'sci.1.90.1_api.43.1.0' into bc-refactor-patchindexing

3a32a3a

Refactors testing directory Also makes a minor error message update to the quadratic solver

fix bad merge

9bcf1cf

manually revert bc_in back to non-array

336b87f

		subroutine SetLastState(this, last_state)

		! This procedure sets the last state logical metadata for the calling interface variable

Conversation

glemieux commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Collaborators:

Expectation of Answer Changes:

Checklist

Documentation

Test Results:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glemieux Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ckoven left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glemieux commented Sep 30, 2025 •

edited

Loading

glemieux Feb 25, 2026 •

edited

Loading