Parameterizations of sub-grid topographic effects on solar radiation in ELM #4590

daleihao · 2021-10-08T15:36:23Z

A well-validated sub-grid topographic (TOP) parameterization is added in ELM
that quantifies the effects of sub-grid topography on solar radiation flux
including the shadow effects and multi-scattering between adjacent terrain.
The new parameterization can be turned on by use_top_solar_rad = .true.
A test for the new parameterization is added.

[BFB]

rljacob · 2021-10-11T20:24:17Z

@daleihao how long does this new test take to run?
SMS_Lm1.f09_f09.IELM.elm-solar_rad

Does it need to run for 1 month? If so, we might have to move it to extra_coverage.

daleihao · 2021-10-11T22:09:05Z

Hi Robert, It just takes about 10 minutes. Please see below for the log information of the test: 2021-10-07 15:23:50: case.run starting 48115437 --------------------------------------------------- 2021-10-07 15:24:25: model execution starting 48115437 --------------------------------------------------- 2021-10-07 15:34:54: model execution success 48115437 --------------------------------------------------- 2021-10-07 15:34:54: case.run success 48115437 --------------------------------------------------- Best, -Dalei Robert Jacob ***@***.***> 于2021年10月11日周一下午1:24写道：

…

@daleihao <https://github.com/daleihao> how long does this new test take to run? SMS_Lm1.f09_f09.IELM.elm-solar_rad Does it need to run for 1 month? If so, we might have to move it to extra_coverage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4590 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMSGUOULWP72XAPOF72X3EDUGNBXXANCNFSM5FT4K2FQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

bishtgautam · 2021-10-16T03:37:28Z

@rljacob After additional analysis, I believe the test can be changed to a regular ERS test. Does that sound okay?

rljacob · 2021-10-18T01:59:21Z

You mean a 7 day ERS test? That's fine unless theres a reason it need to run for a month.

bishtgautam · 2021-11-08T16:21:19Z

@rljacob The test has been modified to be an ERS test.

…to next (PR #4590) A well-validated sub-grid topographic (TOP) parameterization is added in ELM that quantifies the effects of sub-grid topography on solar radiation flux including the shadow effects and multi-scattering between adjacent terrain. The new parameterization can be turned on by `use_top_solar_rad = .true.` A test for the new parameterization is added. [BFB]

Re-merging the PR that now includes a fix for the build error for debug tests.

rljacob · 2021-11-11T19:22:27Z

The ERS test for this feature is failing on gnu and on pgi:
ERS.f09_f09.IELM.mappy_gnu.elm-solar_rad
ERS.f09_f09.IELM.compy_pgi.elm-solar_rad

SUMMARY of cprnc (from compy)
A total number of 193 fields were compared
of which 37 had non-zero differences
and 0 had differences in fill patterns
A total number of 0 fields could not be analyzed
A total number of 0 fields on file 1 were not found on file2.
diff_test: the two files seem to be DIFFERENT

oddly, its passing on Chrysalis with intel.

@bishtgautam I force-pushed next to remove your fix from this morning since we may have to revert the whole PR. This way, you'll only have to revert one thing from next.

bishtgautam · 2021-11-11T21:03:46Z

Thanks @rljacob for the force-pushed

…_top' into next (PR #4590)" This reverts commit 4a23da5, reversing changes made to 57c9480.

bishtgautam

@daleihao The PR needs code cleanup related to comments and indentation.

components/elm/src/biogeophys/SurfaceAlbedoMod.F90

components/elm/src/main/surfrdMod.F90

components/elm/src/utils/domainMod.F90

components/elm/src/biogeophys/SurfaceAlbedoMod.F90

bishtgautam

@daleihao The changes look great. I have have few additional minor changes that I forgot to ask you to fix in my first review.

components/elm/src/biogeophys/SurfaceAlbedoMod.F90

…olar_rad_top' into next (PR #4590)"" This reverts commit c19957a.

…ith-pr-4590 (PR #4590) The branch is re-merged to next

…olar_rad_top' into next (PR #4590)"" This reverts commit c19957a.

The PR-#4590 is merged again.

rljacob · 2021-12-23T18:28:35Z

This was reverted from next because it had unexpected diffs. Please fix.

rljacob · 2022-01-27T18:32:30Z

@daleihao have you fixed the unexpected diffs?

daleihao · 2022-01-30T03:18:15Z

@daleihao have you fixed the unexpected diffs?

I have checked the code. All the tests passed the regression test. Hope it works now.

rljacob · 2022-02-24T18:41:43Z

@bishtgautam ready to try merging this again?

bishtgautam · 2022-02-25T15:59:05Z

@rljacob If next is open today, I will merge this PR today.

rljacob · 2022-02-25T18:09:10Z

@bishtgautam next is open today.

A well-validated sub-grid topographic (TOP) parameterization is added in ELM that quantifies the effects of sub-grid topography on solar radiation flux including the shadow effects and multi-scattering between adjacent terrain. The new parameterization can be turned on by use_top_solar_rad = .true. A test for the new parameterization is added. [BFB]

bishtgautam · 2022-02-25T18:50:43Z

Merged to next

This reverts commit 0514676, reversing changes made to 2a92ba1.

Solve indent issues

#4590)"" This reverts commit 20fe1ff.

The PR is re-merged after the developer has fixed bugs for PGI.

Re-merging the PR with a new commit that removes the two new fields that were added to ELM history file.

amametjanov · 2022-04-08T18:26:56Z

components/elm/cime_config/testdefs/testmods_dirs/elm/solar_rad/shell_commands

@@ -0,0 +1 @@
+./xmlchange NTASKS=48


@daleihao is there a reason to set NTASKS=48 ? With this, ERS.f09_f09.IELM.ascent_pgi.elm-solar_rad is erroring out in 2nd/restart run with memory corruption

2: 14: 0: ALLOCATE: 2356269769344 bytes requested; not enough memory 2: 42: 0: ALLOCATE: 5442822862464 bytes requested; not enough memory

Changing NTASKS to default 1-node with 84 NTASKS lets both initial and restart runs complete BFB. Okay to remove this mod?

It doesn't matter what NTASKs is. It is OK to remove it.

rljacob requested a review from tktesfa2014 October 8, 2021 17:38

rljacob assigned peterdschwartz Oct 8, 2021

rljacob added the Land label Oct 8, 2021

rljacob added this to the v2.1alpha milestone Oct 11, 2021

rljacob added the BFB PR leaves answers BFB label Oct 11, 2021

daleihao force-pushed the daleihao/lnd/solar_rad_top branch from 864fc6e to 962782b Compare October 16, 2021 00:20

daleihao force-pushed the daleihao/lnd/solar_rad_top branch from 962782b to d79a6a1 Compare October 16, 2021 03:56

bishtgautam requested a review from rljacob November 8, 2021 16:21

rljacob approved these changes Nov 8, 2021

View reviewed changes

rljacob assigned bishtgautam and unassigned peterdschwartz Nov 10, 2021

bishtgautam pushed a commit that referenced this pull request Nov 11, 2021

Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)

8b4db09

Re-merging the PR that now includes a fix for the build error for debug tests.

bishtgautam added a commit that referenced this pull request Nov 12, 2021

Revert "Merge remote-tracking branch 'daleihao/daleihao/lnd/solar_rad…

c19957a

…_top' into next (PR #4590)" This reverts commit 4a23da5, reversing changes made to 57c9480.

bishtgautam requested changes Nov 26, 2021

View reviewed changes

bishtgautam reviewed Dec 7, 2021

View reviewed changes

components/elm/src/biogeophys/SurfaceAlbedoMod.F90 Outdated Show resolved Hide resolved

components/elm/src/biogeophys/SurfaceAlbedoMod.F90 Outdated Show resolved Hide resolved

components/elm/src/biogeophys/SurfaceAlbedoMod.F90 Outdated Show resolved Hide resolved

bishtgautam self-requested a review December 15, 2021 22:54

bishtgautam approved these changes Dec 15, 2021

View reviewed changes

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Revert "Revert "Merge remote-tracking branch 'daleihao/daleihao/lnd/s…

1799a7b

…olar_rad_top' into next (PR #4590)"" This reverts commit c19957a.

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)

63de151

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Revert "Revert "Merge remote-tracking branch 'daleihao/daleihao/lnd/s…

b882728

…olar_rad_top' into next (PR #4590)"" This reverts commit c19957a.

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Merge branch 'daleihao/lnd/solar_rad_top' into bishtgautam/lnd/next-w…

2923a70

…ith-pr-4590 (PR #4590) The branch is re-merged to next

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Revert "Revert "Merge remote-tracking branch 'daleihao/daleihao/lnd/s…

864c528

…olar_rad_top' into next (PR #4590)"" This reverts commit c19957a.

bishtgautam added a commit that referenced this pull request Dec 15, 2021

Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)

5d9ccba

The PR-#4590 is merged again.

add solar radiation for top

93bda58

daleihao force-pushed the daleihao/lnd/solar_rad_top branch 3 times, most recently from 1acc227 to 93bda58 Compare January 30, 2022 02:54

update surfdata filename for solar_rad test

9ec0dfd

bishtgautam added a commit that referenced this pull request Feb 28, 2022

Revert "Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)"

20fe1ff

This reverts commit 0514676, reversing changes made to 2a92ba1.

Dalei Hao and others added 3 commits March 13, 2022 12:18

Update for PGI

a0cac09

Solve indent issues

5f40fec

Solve indent issues

debug: end -> endif

60536be

bishtgautam added a commit that referenced this pull request Mar 14, 2022

Revert "Revert "Merge branch 'daleihao/lnd/solar_rad_top' into next (PR

fcc5589

#4590)"" This reverts commit 20fe1ff.

bishtgautam added a commit that referenced this pull request Mar 14, 2022

Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)

ec8ac40

The PR is re-merged after the developer has fixed bugs for PGI.

remove added history output

cb6ee45

bishtgautam added a commit that referenced this pull request Mar 15, 2022

Merge branch 'daleihao/lnd/solar_rad_top' into next (PR #4590)

d7cd150

Re-merging the PR with a new commit that removes the two new fields that were added to ELM history file.

bishtgautam merged commit d0663d4 into E3SM-Project:master Mar 16, 2022

bishtgautam deleted the daleihao/lnd/solar_rad_top branch March 17, 2022 16:58

amametjanov reviewed Apr 8, 2022

View reviewed changes

rljacob added the Stealth PR has feature which, if turned on, could change climate. fka FCC label Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameterizations of sub-grid topographic effects on solar radiation in ELM #4590

Parameterizations of sub-grid topographic effects on solar radiation in ELM #4590

daleihao commented Oct 8, 2021

rljacob commented Oct 11, 2021

daleihao commented Oct 11, 2021 via email

bishtgautam commented Oct 16, 2021

rljacob commented Oct 18, 2021

bishtgautam commented Nov 8, 2021

rljacob commented Nov 11, 2021

bishtgautam commented Nov 11, 2021

bishtgautam left a comment

bishtgautam left a comment

rljacob commented Dec 23, 2021

rljacob commented Jan 27, 2022

daleihao commented Jan 30, 2022

rljacob commented Feb 24, 2022

bishtgautam commented Feb 25, 2022

rljacob commented Feb 25, 2022

bishtgautam commented Feb 25, 2022

amametjanov Apr 8, 2022

daleihao Apr 8, 2022

Parameterizations of sub-grid topographic effects on solar radiation in ELM #4590

Parameterizations of sub-grid topographic effects on solar radiation in ELM #4590

Conversation

daleihao commented Oct 8, 2021

rljacob commented Oct 11, 2021

daleihao commented Oct 11, 2021 via email

bishtgautam commented Oct 16, 2021

rljacob commented Oct 18, 2021

bishtgautam commented Nov 8, 2021

rljacob commented Nov 11, 2021

bishtgautam commented Nov 11, 2021

bishtgautam left a comment

Choose a reason for hiding this comment

bishtgautam left a comment

Choose a reason for hiding this comment

rljacob commented Dec 23, 2021

rljacob commented Jan 27, 2022

daleihao commented Jan 30, 2022

rljacob commented Feb 24, 2022

bishtgautam commented Feb 25, 2022

rljacob commented Feb 25, 2022

bishtgautam commented Feb 25, 2022

amametjanov Apr 8, 2022

Choose a reason for hiding this comment

daleihao Apr 8, 2022

Choose a reason for hiding this comment