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

fix area scaling to correct RTM river flux to MOM6 #51

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

swensosc
Copy link
Contributor

@swensosc swensosc commented Jun 5, 2024

fix how area is applied to input runoff for rtm

Fixes #50

@ekluzek ekluzek added this to the CESM3.0 milestone Jun 5, 2024
@ekluzek ekluzek requested review from ekluzek and slevis-lmwg June 5, 2024 18:29
@wwieder wwieder changed the title fix area scaling fix area scaling to correct RTM river flux to MOM6 Jun 17, 2024
@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jun 19, 2024

Test-suite to generate baselines on derecho:
./run_sys_tests -s rtm -c rtm1_0_79_ctsm5.2.0 -g rtm1_0_79_ctsm5.2.007
I see NLCOMP failures, which do not seem surprising to me.

I have checked out this branch and submitted the test-suite against the baselines:
./run_sys_tests -s rtm -c rtm1_0_79_ctsm5.2.007 -g rtm1_0_80_ctsm5.2.007

The naming convention is correct, it hasn't changed for the RTM as it has for MOSART.

Also, I tried testing on izumi and found out that we have no rtm tests on izumi.

@slevis-lmwg
Copy link
Contributor

The only failures are DIFF against the baseline, which seems expected, right?

I see diffs in QCHANR, QCHANR_ICE, QCHOCNR, QCHOCNR_ICE
and
lndExp_Flrr_volr, lndExp_Flrr_volrmch, rofImp_Flrr_volr, rofImp_Flrr_volrmch, rofImp_Forr_rofi, rofImp_Forr_rofl, rofExp_Flrl_irrig

Results here:
/glade/work/slevis/git_externals/rtm_fix/tests_0618-182038de

@swensosc
Copy link
Contributor Author

swensosc commented Jun 19, 2024 via email

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jun 19, 2024

from meeting with @ekluzek

FAIL Try making an mct baseline with ctsm5.2.005 (see posts below about the failure):
./run_sys_tests -s rtm -c rtm1_0_79_ctsm5.2.0 -g rtm1_0_79_mct_ctsm5.2.005 --extra-create-test-args " --driver mct"
and then comparing to it:
./run_sys_tests -s rtm -c rtm1_0_79_mct_ctsm5.2.005 --skip-generate

  • @slevis-lmwg compare the code changes against what was in rtm mct (so an older version of rtm)
    In /glade/work/slevis/git_ctsm_tags/ctsm5.1.dev154/components/rtm/src/cpl/mct/rof_import_export.F90
    I see area multiplied by 0.001_r8 (as Sean does in this PR), except when the area divides volr.
  • @slevis-lmwg compare what the nuopc cap looks like in mosart (should be doing the same thing)
    In /glade/work/slevis/git_externals/rtm_fix/components/mosart/src/cpl/nuopc/rof_import_export.F90
    I see every instance of area multiplied by 0.001_r8 (as Sean does in this PR).
    I see the same line that Sean now corrected in this PR (line 200).

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jun 19, 2024

@ekluzek I tried our first TODO with ctsm5.2.005 as well as with ctsm5.2.0 and got this error in all the tests:

SETUP FAILED for test 'SMS_Ld5_Vmct.f10_f10_mg37.I2000Clm50BgcCropRtm.derecho_intel.rtm-rtmColdStart'.
Command: ./case.setup
Output:
ERROR: No variable BUILD_THREADED found in case

@slevis-lmwg
Copy link
Contributor

I think the problem with testing mct is that it was removed from rtm long before these versions. From a sampling of tags that I have in my directories:

  • dev154 has mct
  • dev173 doesn't

Copy link
Contributor

@slevis-lmwg slevis-lmwg left a comment

Choose a reason for hiding this comment

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

My comments appear in a separate post.

@slevis-lmwg
Copy link
Contributor

@ekluzek I will give you a chance to voice further concerns. I will plan on merging tomorrow afternoon.

@slevis-lmwg slevis-lmwg merged commit b3dfcfb into ESCOMP:master Jun 21, 2024
@slevis-lmwg
Copy link
Contributor

Made new tag:

git tag -a rtm1_0_80
git push escomp rtm1_0_80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Likely wrong RTM river flux to MOM6 within cesm2_3_beta17
3 participants