Update ELM-FATES API to pass fire data to FATES #5369

glemieux · 2022-12-19T17:48:11Z

This pull request updates the ELM-FATES API to provide FATES with the lightning and population density data from FireMod.F90. This provides ELM-FATES users access to the additional SPITFIRE run modes. The design is adapts the framework developed for CLM-FATES. The design document discussing the background and general design is available in the FATES Developer's Guide. All non-fates tests should be b4b as this PR only adds access to additional FATES modes which are not yet covered by existing tests.

This also updates the fates pointer to tag sci.1.63.2_api.25.1.0 bring in the fix to the cray and ibm pointer deallocation issue to resolve

Fixes #5001.
[B4B]

glemieux · 2023-02-16T18:16:33Z

Removed WIP. Testing TBD this week.

components/elm/bld/namelist_files/namelist_definition.xml

components/elm/src/main/ndepStreamMod.F90

rljacob · 2023-03-02T18:43:25Z

@rgknox waiting for you to approve.

glemieux · 2023-03-02T19:11:37Z

@rgknox waiting for you to approve.

@rgknox just a heads up that @JessicaNeedham has a small update she wants to add to this PR. It should be orthogonal to the main thrust of this PR and is a partial solution to the issue discussed in NGEET/fates#985. The change is to update use_fates_planthydro to use_fates here.

rgknox · 2023-03-02T19:21:31Z

@glemieux, I will wait until the final changes are in. Can you ping me when its ready?

glemieux · 2023-03-02T19:24:45Z

@glemieux, I will wait until the final changes are in. Can you ping me when its ready?

Will do. Jessie is testing this out the changes.

JessicaNeedham · 2023-03-02T19:29:41Z

Thanks @glemieux. I'm finding that establishment from bare ground works better when we start with higher initial soil moisture. I wondered about changing the use_fates_planthydro to use_fates here:

E3SM/components/elm/src/data_types/ColumnDataType.F90

Line 1619 in b30a6a0

if (use_fates_planthydro .or. use_hydrstress) then

so that soil moisture is always initialised high when FATES is on.
Details are here NGEET/fates#985 although maybe I should start a separate issue? I believe @mpaiao has also found it necessary to increase soil moisture.
I haven't tested yet on this branch but I will do so now.

glemieux · 2023-03-02T19:48:04Z

Details are here NGEET/fates#985 although maybe I should start a separate issue? I believe @mpaiao has also found it necessary to increase soil moisture.
I haven't tested yet on this branch but I will do so now.

@JessicaNeedham We could convert the discussion to an issue if you feel like it has been fully addressed. If there are still outstanding issues in the discussion then opening a new issue (and linking back to the open discussion) would be better.

Skimming the discussion, are there changes on the fates-side that need to be made as well? If so, it might be cleaner to update and integrate the fates-side first and then create new PRs for both host land model side.

JessicaNeedham · 2023-03-02T20:07:42Z

Thanks @glemieux, I'd say it is not totally resolved so I'll make a new issue for some of the solutions with a corresponding FATES PR.

rgknox · 2023-03-02T21:54:23Z

I agree @glemieux, it might be better to add those changes in another PR

glemieux · 2023-03-03T21:37:28Z

@rgknox we're going to do a different PR for Jessie's changes.

I ran into a run time seg fault in the testing of spitfire in mode 2 right now. I'll ping you again when I've got things fixed up.

This imports and adapts the clm-fates fire base types and factory method of fire type creation. The standard elm FireMod has not been changed, but could adopt this method in the future if so desired. FATES will now have access to the same lightning and population data the the FireMod make use of. The code also includes the hooks to enable the use of additonal FATES fire modes, but these require the use of data sets not currently stored on most machines.

glemieux · 2023-03-06T22:45:29Z

@peterdschwartz I'm running regression testing on perlmutter currently. I'll update with the results as soon as possible.

glemieux · 2023-03-07T17:24:33Z

@peterdschwartz testing on perlmutter against the master baseline is complete. All tests are B4B. There are some failures due to namelist and compute time differences:

All fates tests have NLCOMP diffs as they include access to the lightning and pop dens data
2 non-fates tests have TPUTCOMP increases

Folder location: /pscratch/sd/g/glemieux/e3sm-tests/pr5369.fates.pm-cpu.gnu.C41339b97c9-Fb8b905cc

components/elm/src/biogeochem/FireDataBaseType.F90

This commit also cleans up some comments

glemieux · 2023-03-09T01:12:33Z

@peterdschwartz just a heads up that I attempted to run land developer tests on summit since perlmutter is down, but I ran into a model build issue. Since this didn't come up previously, I'm guessing its a compiler-specific issue that I'll need to address.

The IBM compiler requires a return assignment for functions.

glemieux · 2023-03-10T19:53:50Z

The issue I was seeing on summit has been resolved with c94350c. The problem was that the ibm compiler didn't like that I wasn't setting a return value for two empty functions. For posterity the form of the error was:

"/autofs/nccs-svm1_home1/glemieux/e3sm/components/elm/src/biogeochem/FATESFireNoDataMod.F90", 1513-083 (E) Internal or module function lnfm24 was not set within the function.

The issue here is that the function is the intentionally empty form of a polymorphic procedure that is defined to end the run with an error message. It's setup this was to make sure that the call to the function isn't applied in an instance where it is not allowed. Intel and gnu compilers didn't seem to have a problem with this.

Final land developer testing is underway again.

glemieux · 2023-03-10T21:36:15Z

Land developer tests are complete. Comparison against master on perlmutter is B4B with the exception of the expected NLCOMP differences for the fates tests. I ran the tests on summit as well (all PASS), although I wasn't able to compare it against the master baseline for some reason.

Folder on perlmutter: /pscratch/sd/g/glemieux/e3sm-tests/pr5369-ibmfix.fates.pm-cpu.gnu.Cc94350cd0a-Fb8b905cc
Folder on summit: /gpfs/alpine/cli115/scratch/glemieux/e3sm-tests/pr5369-ibmfix.fates.summit.ibm.Cc94350cd0a-Fb8b905cc

…pi' into next (PR #5369) This pull request updates the ELM-FATES API to provide FATES with the lightning and population density data from FireMod.F90. This provides ELM-FATES users access to the additional SPITFIRE run modes. The design is adapts the framework developed for CLM-FATES. The design document discussing the background and general design is available in the FATES Developer's Guide. All non-fates tests should be b4b as this PR only adds access to additional FATES modes which are not yet covered by existing tests. This also updates the fates pointer to tag sci.1.63.2_api.25.1.0 bring in the fix to the cray and ibm pointer deallocation issue to resolve #5001. Fixes #5001 [B4B]

peterdschwartz · 2023-03-22T17:48:08Z

merged to next

peterdschwartz · 2023-03-27T15:34:12Z

merged to master

grnydawn · 2023-10-30T16:27:01Z

This PR is already merged but I put the following bug report from one of ORNL helpdesk team in regards to this issue:

The deallocate(nextc) call throws the error because nextc is an associated pointer and was never allocated itself. I was using the ERS_Ld20.f45_f45.IELMFATES.crusher_crayclang.elm-fates test case to trigger this. The $E3SM/components/elm/src/external_models/fates/biogeochem/EDCohortDynamicsMod.F90 file is what I was looking at, as well. The following lines from that file are relevant:
subroutine fuse_cohorts(currentSite, currentPatch, bc_in)
...
type (ed_patch_type), intent(inout), target :: currentPatch
...
type (ed_cohort_type) , pointer :: nextc
...
do while(iterate == 1)
currentCohort => currentPatch%tallest
...
do while ( .not.associated(currentCohort,currentPatch%shortest) )
nextc => currentPatch%tallest
do while (associated(nextc))
...
! deallocates the members of nextc
call DeallocateCohort(nextc)
deallocate(nextc) -- this fails!

rgknox · 2023-10-30T18:41:05Z

Thanks for posting @grnydawn

This seems related to this previous issue you created: #5001

Some compilers were having trouble allocating and deallocating these derived types, one of the reasons was the "target" designation. I thought we implemented a fix, in a more updated version of the code below, we don't have the target designation.

https://github.com/NGEET/fates/blob/main/biogeochem/EDCohortDynamicsMod.F90#L710-L712

Have you tried your tests with the current master version of E3SM? That should point to a more modern fates tag.

grnydawn · 2023-10-30T20:33:37Z

@rgknox , No, I have not tried the tests with the latest master branch recently. I will add comment here if I still have this issue.

rljacob assigned peterdschwartz Dec 23, 2022

rljacob added Land FATES labels Jan 10, 2023

rljacob requested a review from rgknox January 10, 2023 21:56

glemieux changed the title ~~[WIP] Update ELM-FATES API to pass fire data to FATES~~ Update ELM-FATES API to pass fire data to FATES Feb 16, 2023

rgknox reviewed Feb 28, 2023

View reviewed changes

components/elm/bld/namelist_files/namelist_definition.xml Outdated Show resolved Hide resolved

rgknox reviewed Feb 28, 2023

View reviewed changes

components/elm/src/main/ndepStreamMod.F90 Show resolved Hide resolved

rgknox approved these changes Mar 2, 2023

View reviewed changes

glemieux force-pushed the glemieux/lnd/fates-fire-data-api branch from bad2687 to 41339b9 Compare March 6, 2023 22:41

peterdschwartz reviewed Mar 7, 2023

View reviewed changes

components/elm/src/biogeochem/FireDataBaseType.F90 Outdated Show resolved Hide resolved

Change nan to spval for elm-fates fire data

ee63a0a

This commit also cleans up some comments

glemieux changed the title ~~Update ELM-FATES API to pass fire data to FATES~~ [WIP] Update ELM-FATES API to pass fire data to FATES Mar 9, 2023

Add function result assignment for nofiredata type

c94350c

The IBM compiler requires a return assignment for functions.

glemieux changed the title ~~[WIP] Update ELM-FATES API to pass fire data to FATES~~ Update ELM-FATES API to pass fire data to FATES Mar 10, 2023

This was referenced Mar 22, 2023

Test elm-fates fire data update over all modes glemieux/E3SM#19

Closed

Update ELM code to pass fire-related data to FATES glemieux/E3SM#14

Closed

peterdschwartz merged commit 2f5c01d into E3SM-Project:master Mar 27, 2023

mahf708 mentioned this pull request May 1, 2023

reduce opt on elm files #5650

Merged

glemieux mentioned this pull request Jul 19, 2023

Add option to hist_addfld call to convert NaN's to spval? ESCOMP/CTSM#2064

Closed

glemieux deleted the glemieux/lnd/fates-fire-data-api branch January 23, 2024 23:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ELM-FATES API to pass fire data to FATES #5369

Update ELM-FATES API to pass fire data to FATES #5369

glemieux commented Dec 19, 2022 •

edited by peterdschwartz

Loading

glemieux commented Feb 16, 2023

rljacob commented Mar 2, 2023

glemieux commented Mar 2, 2023 •

edited

Loading

rgknox commented Mar 2, 2023

glemieux commented Mar 2, 2023

JessicaNeedham commented Mar 2, 2023

glemieux commented Mar 2, 2023

JessicaNeedham commented Mar 2, 2023

rgknox commented Mar 2, 2023

glemieux commented Mar 3, 2023

glemieux commented Mar 6, 2023 •

edited

Loading

glemieux commented Mar 7, 2023 •

edited

Loading

glemieux commented Mar 9, 2023

glemieux commented Mar 10, 2023

glemieux commented Mar 10, 2023 •

edited

Loading

peterdschwartz commented Mar 22, 2023

peterdschwartz commented Mar 27, 2023

grnydawn commented Oct 30, 2023

rgknox commented Oct 30, 2023

grnydawn commented Oct 30, 2023

Update ELM-FATES API to pass fire data to FATES #5369

Update ELM-FATES API to pass fire data to FATES #5369

Conversation

glemieux commented Dec 19, 2022 • edited by peterdschwartz Loading

glemieux commented Feb 16, 2023

rljacob commented Mar 2, 2023

glemieux commented Mar 2, 2023 • edited Loading

rgknox commented Mar 2, 2023

glemieux commented Mar 2, 2023

JessicaNeedham commented Mar 2, 2023

glemieux commented Mar 2, 2023

JessicaNeedham commented Mar 2, 2023

rgknox commented Mar 2, 2023

glemieux commented Mar 3, 2023

glemieux commented Mar 6, 2023 • edited Loading

glemieux commented Mar 7, 2023 • edited Loading

glemieux commented Mar 9, 2023

glemieux commented Mar 10, 2023

glemieux commented Mar 10, 2023 • edited Loading

peterdschwartz commented Mar 22, 2023

peterdschwartz commented Mar 27, 2023

grnydawn commented Oct 30, 2023

rgknox commented Oct 30, 2023

grnydawn commented Oct 30, 2023

glemieux commented Dec 19, 2022 •

edited by peterdschwartz

Loading

glemieux commented Mar 2, 2023 •

edited

Loading

glemieux commented Mar 6, 2023 •

edited

Loading

glemieux commented Mar 7, 2023 •

edited

Loading

glemieux commented Mar 10, 2023 •

edited

Loading