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

Adds MOSART-heat #3133

Merged
merged 18 commits into from
Nov 27, 2019
Merged

Adds MOSART-heat #3133

merged 18 commits into from
Nov 27, 2019

Conversation

liho745
Copy link
Contributor

@liho745 liho745 commented Aug 15, 2019

Added a new submodule to MOSART, MOSART-heat, which simulates water
temperature in the rivers. MOSART-heat is coupled to both ELM and EAM.
More specifically, MOSART-heat needs the runoff and soil temperature
simulated by ELM, and solar radiation, wind speed etc. from EAM.
To couple MOSART-heat with EAM, we newly added the ATM2ROF
mapping (including code changes and several new ATM2ROF mapping files)
which did not exist previously in E3SM. MOSART-heat can be turned on/off
with a heatflag option in user_nl_mosart.

[BFB]
[NML]

Copy link
Member

@rljacob rljacob left a comment

Choose a reason for hiding this comment

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

Remove the config_machines change.
Edit the top level description to better describe what is being added.
What is all the atm2rof mapping stuff for?
The rrtmpg submodule should not be changed.

@liho745
Copy link
Contributor Author

liho745 commented Aug 16, 2019

I've reverted the config_machines change.
I edited the top-level description with more details, including why adding ATM2ROF mapping.
I did not, however, change rrtmpg submodule though. Perhaps I did not update all the sub-modules before committing my changes? In any case, I agree it should not be changed.

@thurber
Copy link
Contributor

thurber commented Aug 20, 2019

i pushed a new commit that reverts the rrtmpg submodule back it's status in master

Copy link
Member

@rljacob rljacob left a comment

Choose a reason for hiding this comment

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

Github is still showing a config_machines change but its to constance so not a blocker.

Also edit the title of the PR to describe the change.

@rljacob rljacob added this to the v2.0BGC milestone Aug 22, 2019
Copy link
Contributor

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

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

Please make the relevant changes.

Also, make the changes as suggested by @rljacob in the above comment.

components/mosart/src/riverroute/MOSART_heat_mod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/MOSART_heat_mod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/MOSART_heat_mod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/MOSART_heat_mod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/RtmMod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/RtmMod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/RtmMod.F90 Outdated Show resolved Hide resolved
components/mosart/src/riverroute/RtmMod.F90 Outdated Show resolved Hide resolved
cime/src/drivers/mct/main/prep_rof_mod.F90 Outdated Show resolved Hide resolved
@liho745 liho745 changed the title Hongyili/mosart/mosart heat2 Adding the stream temperature module, MOSART-heat, into E3SM v2 Aug 26, 2019
@bishtgautam bishtgautam changed the title Adding the stream temperature module, MOSART-heat, into E3SM v2 Adds MOSART-heat Aug 28, 2019
@thurber
Copy link
Contributor

thurber commented Sep 23, 2019

@liho745, @bishtgautam, any remaining next steps to get this merged?

@bishtgautam
Copy link
Contributor

@thurber, The MPAS submodule has been incorrectly updated in this PR and there are a bunch of conflicts. Once #3074 is merged to master, this PR will be rebased off the latest master.

@bishtgautam
Copy link
Contributor

Thanks, @jonbob

@rljacob
Copy link
Member

rljacob commented Oct 4, 2019

@golaz do you want this for v2 watercycle?

@golaz
Copy link
Contributor

golaz commented Oct 9, 2019

If it's BFB, it could be merged in the code. But turning it on in v2 will depend on someone volunteering to make a simulation with it and explaining the benefits of the feature (and demonstrating no degradation).

bishtgautam added a commit that referenced this pull request Oct 22, 2019
… (PR #3133)""

This reverts commit e586f2f.

Conflicts:
	components/clm/src/cpl/lnd_import_export.F90
@dqwu
Copy link
Contributor

dqwu commented Oct 23, 2019

@bishtgautam
This PR has caused 21 tests failed with latest next branch:
https://my.cdash.org/viewTest.php?onlyfailed&buildid=1719105

I have manually run ERS.f19_f19.ICLM45 on anlworkstation, and the stack trace is:

[0] Program received signal SIGSEGV: Segmentation fault - invalid memory reference.
[0] 
[0] Backtrace for this error:
[0] #0  0x7f2c5b2becaf in ???
[0] #1  0x11e7da9 in __mosart_physics_mod_MOD_euler
[0] 	at components/mosart/src/riverroute/MOSART_physics_mod.F90:133
[0] #2  0x1174c64 in __rtmmod_MOD_rtmrun
[0] 	at components/mosart/src/riverroute/RtmMod.F90:2246
[0] #3  0x114c50b in __rof_comp_mct_MOD_rof_run_mct
[0] 	at components/mosart/src/cpl/rof_comp_mct.F90:361
[0] #4  0x427451 in __component_mod_MOD_component_run
[0] 	at cime/src/drivers/mct/main/component_mod.F90:714
[0] #5  0x4115b4 in __cime_comp_mod_MOD_cime_run
[0] 	at cime/src/drivers/mct/main/cime_comp_mod.F90:2640
[0] #6  0x42611f in cime_driver
[0] 	at cime/src/drivers/mct/main/cime_driver.F90:133
[0] #7  0x42623d in main
[0] 	at cime/src/drivers/mct/main/cime_driver.F90:23

@bishtgautam
Copy link
Contributor

I'm reverting this PR again and will ask the developers to dig into these errors.

bishtgautam added a commit that referenced this pull request Oct 23, 2019
@jonbob
Copy link
Contributor

jonbob commented Nov 25, 2019

With the recent commit, a test merge now passes:

  • SMS_D.ne4_oQU240.A_WCYCL1850.anvil_gnu
  • ERS.f19_f19.ICLM45.anvil_gnu

It may make sense to rebase to master since the merge has conflicts. I can work on that next.

@jonbob
Copy link
Contributor

jonbob commented Nov 26, 2019

@bishtgautam - it may be easier to simply merge this PR instead of rebasing it. Do you want me to take it on? I think it does now pass some of the tests that had been failing

@bishtgautam
Copy link
Contributor

I agree with you regarding rebasing this PR. I will (re-re-)merge this PR to next later today and double check my work before pushing it to next.

@jonbob
Copy link
Contributor

jonbob commented Nov 26, 2019

@bishtgautam - sounds good, and thanks

bishtgautam added a commit that referenced this pull request Nov 26, 2019
bishtgautam added a commit that referenced this pull request Nov 26, 2019
Added a new submodule to MOSART, MOSART-heat, which simulates water
temperature in the rivers. MOSART-heat is coupled to both ELM and EAM.
More specifically, MOSART-heat needs the runoff and soil temperature
simulated by ELM, and solar radiation, wind speed etc. from EAM.
To couple MOSART-heat with EAM, we newly added the ATM2ROF
mapping (including code changes and several new ATM2ROF mapping files)
which did not exist previously in E3SM. MOSART-heat can be turned on/off
with a heatflag option in user_nl_mosart.

[BFB]
[NML]
bishtgautam added a commit that referenced this pull request Nov 27, 2019
Added a new submodule to MOSART, MOSART-heat, which simulates water
temperature in the rivers. MOSART-heat is coupled to both ELM and EAM.
More specifically, MOSART-heat needs the runoff and soil temperature
simulated by ELM, and solar radiation, wind speed etc. from EAM.
To couple MOSART-heat with EAM, we newly added the ATM2ROF
mapping (including code changes and several new ATM2ROF mapping files)
which did not exist previously in E3SM. MOSART-heat can be turned on/off
with a heatflag option in user_nl_mosart.

[BFB]
[NML]

Conflicts:
cime/config/e3sm/machines/config_machines.xml
cime/src/drivers/mct/main/prep_rof_mod.F90
cime/src/drivers/mct/shr/seq_flds_mod.F90
components/clm/src/cpl/clm_cpl_indices.F90
components/clm/src/cpl/lnd_import_export.F90
components/mosart/bld/build-namelist
components/mosart/src/cpl/rof_comp_esmf.F90
components/mosart/src/cpl/rof_comp_mct.F90
components/mosart/src/cpl/rof_cpl_indices.F90
components/mosart/src/riverroute/MOSART_physics_mod.F90
components/mosart/src/riverroute/RtmHistFlds.F90
components/mosart/src/riverroute/RtmMod.F90
@bishtgautam bishtgautam merged commit 51c291b into master Nov 27, 2019
@bishtgautam
Copy link
Contributor

Thanks to @jonbob for fixing the bug found by gnu, this PR has been merged to master.

@hydrotian
Copy link
Contributor

Bravo! Thanks to @jonbob and @bishtgautam !

@liho745
Copy link
Contributor Author

liho745 commented Nov 27, 2019 via email

bishtgautam added a commit that referenced this pull request Dec 2, 2019
Fixes difference in conflict resolution from #3133
rljacob pushed a commit that referenced this pull request Apr 12, 2021
Added a new submodule to MOSART, MOSART-heat, which simulates water
temperature in the rivers. MOSART-heat is coupled to both ELM and EAM.
More specifically, MOSART-heat needs the runoff and soil temperature
simulated by ELM, and solar radiation, wind speed etc. from EAM.
To couple MOSART-heat with EAM, we newly added the ATM2ROF
mapping (including code changes and several new ATM2ROF mapping files)
which did not exist previously in E3SM. MOSART-heat can be turned on/off
with a heatflag option in user_nl_mosart.

[BFB]
[NML]

Conflicts:
cime/config/e3sm/machines/config_machines.xml
cime/src/drivers/mct/main/prep_rof_mod.F90
cime/src/drivers/mct/shr/seq_flds_mod.F90
components/clm/src/cpl/clm_cpl_indices.F90
components/clm/src/cpl/lnd_import_export.F90
components/mosart/bld/build-namelist
components/mosart/src/cpl/rof_comp_esmf.F90
components/mosart/src/cpl/rof_comp_mct.F90
components/mosart/src/cpl/rof_cpl_indices.F90
components/mosart/src/riverroute/MOSART_physics_mod.F90
components/mosart/src/riverroute/RtmHistFlds.F90
components/mosart/src/riverroute/RtmMod.F90
rljacob pushed a commit that referenced this pull request Apr 21, 2021
Added a new submodule to MOSART, MOSART-heat, which simulates water
temperature in the rivers. MOSART-heat is coupled to both ELM and EAM.
More specifically, MOSART-heat needs the runoff and soil temperature
simulated by ELM, and solar radiation, wind speed etc. from EAM.
To couple MOSART-heat with EAM, we newly added the ATM2ROF
mapping (including code changes and several new ATM2ROF mapping files)
which did not exist previously in E3SM. MOSART-heat can be turned on/off
with a heatflag option in user_nl_mosart.

[BFB]
[NML]

Conflicts:
cime/config/e3sm/machines/config_machines.xml
cime/src/drivers/mct/main/prep_rof_mod.F90
cime/src/drivers/mct/shr/seq_flds_mod.F90
components/clm/src/cpl/clm_cpl_indices.F90
components/clm/src/cpl/lnd_import_export.F90
components/mosart/bld/build-namelist
components/mosart/src/cpl/rof_comp_esmf.F90
components/mosart/src/cpl/rof_comp_mct.F90
components/mosart/src/cpl/rof_cpl_indices.F90
components/mosart/src/riverroute/MOSART_physics_mod.F90
components/mosart/src/riverroute/RtmHistFlds.F90
components/mosart/src/riverroute/RtmMod.F90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants