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 DLND to moab coupler #6567

Merged
merged 8 commits into from
Sep 22, 2024
Merged

Add DLND to moab coupler #6567

merged 8 commits into from
Sep 22, 2024

Conversation

iulian787
Copy link
Contributor

@iulian787 iulian787 commented Aug 27, 2024

Add data land capability for moab driver;
case tested: --res r05_r05 --compset RMOSGPCC
standard data moab model, mainly adapt from data ocean implementation

major change: Initialize all factors set by MOAB area correction init routine to 1.0. It affects all models,
but it seems that some values were not initialized for data land case.
On chrysalis, with intel compiler, it actually complained about non initialized values when applying correction factors.

The compressing file errors seen during development were caused by non-ascii characters in the log files.
Those were caused by an uninitialized mapper, for which we were still creating a MOAB map. It was never used,
it is fixed properly now, the moab map is not computed anymore.

[BFB]

use existing lnd_domain attribute in infodata
we do not differentiate on coupler side if data land or not
still need to send the proper land domain file
domain file is actually set, in shr_strdata_type%domainFile
all the other data models use the first stream file for domain
information

land is the one that actually has that member set in the SDLND
structure

it is easier to get it from there, then from stream
the domain file in regular land case is defined by fatmlndfrac file
iMOAB_ApplyScalarProjectionWeights is not used
anymore
all maps are done now with seq_map method, and there is only
one instance of iMOAB_ApplyScalarProjectionWeights in E3SM
using moab driver
only intel on chrysalis complained about it
It could be a cause for trouble later on
Copy link

github-actions bot commented Aug 27, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6567/
on branch gh-pages at 2024-09-12 23:23 UTC

@rljacob rljacob changed the title Iulian787/data land moab Add DLND to moab coupler Aug 27, 2024
@rljacob rljacob self-assigned this Aug 28, 2024
@rljacob rljacob requested a review from vijaysm August 28, 2024 01:17
Copy link
Contributor

@vijaysm vijaysm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the code and made comments. I haven't run this branch and tested it just yet.

components/data_comps/dlnd/src/dlnd_comp_mod.F90 Outdated Show resolved Hide resolved
components/data_comps/dlnd/src/dlnd_comp_mod.F90 Outdated Show resolved Hide resolved
driver-moab/main/component_mod.F90 Show resolved Hide resolved
@iulian787 iulian787 closed this Sep 9, 2024
@iulian787
Copy link
Contributor Author

I tested this branch on different platforms, and chrysalis is working fine; gce, perlmutter, and my laptop (gnu compiler) all fail to compress the log files at the end. So the bug I saw earlier is not solved yet. Somehow, it worked on gce when I tested 2 weeks ago; needs more work

compute rof lnd interaction only if rof_c2_lnd is true
we were computing an unnecessary map always
even if rof_c2_lnd is false
(for most coupled cases it is true)
in case rof c2 lnd, we were not retrieving the the MPI group
needed for comm graph computation, for same grid path
we were retrieving it for intx only
how come we never had samegrid land rof before ?
@iulian787 iulian787 reopened this Sep 10, 2024
Copy link
Contributor

@vijaysm vijaysm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more comments to address

components/data_comps/dlnd/src/dlnd_comp_mod.F90 Outdated Show resolved Hide resolved
driver-moab/main/prep_lnd_mod.F90 Show resolved Hide resolved
@rljacob rljacob added BFB PR leaves answers BFB labels Sep 18, 2024
Copy link
Contributor

@vijaysm vijaysm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my comments!

rljacob added a commit that referenced this pull request Sep 20, 2024
Add data land capability for moab driver;
case tested: --res r05_r05 --compset RMOSGPCC
standard data moab model, mainly adapt from data ocean implementation

major change: Initialize all factors set by MOAB area correction init routine to 1.0. It affects all models,
but it seems that some values were not initialized for data land case.
On chrysalis, with intel compiler, it actually complained about non initialized values when applying correction factors.

The compressing file errors seen during development were caused by non-ascii characters in the log files.
Those were caused by an uninitialized mapper, for which we were still creating a MOAB map. It was never used,
it is fixed properly now, the moab map is not computed anymore.

[BFB]
@rljacob rljacob merged commit 4a44b19 into master Sep 22, 2024
3 checks passed
@rljacob rljacob deleted the iulian787/data_land_moab branch September 22, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Coupler data-models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants