E3SM icon indicating copy to clipboard operation
E3SM copied to clipboard

Published 20 hours ago • verified publisher icon E3SM-Project

MOSART-sediment: suspended sediment module

Open liho745 opened this issue 2 years ago • 7 comments

MOSART-sediment, the suspended sediment module, takes lateral sediment yield fluxes from a land model (such as the soil erosion module in ELM) as the inputs and includes the description of riverine transport, erosion, and deposition processes. It also accounts for two major damming effects on suspended sediment load, direct sediment trapping effect and indirect flow regulating effect (via reservoir regulation of streamflow and hydraulic conditions). The scientific validation was conducted over the NLDAS2 domain (against observations from multiple USGS gages. The current parameter files are for the NLDAS2 domain only, although the process description is general and largely transferrable.

[BFB]

liho745 avatar Aug 01 '22 17:08 liho745

@liho745 Please define 0.95 as a constant variable and provide description.

Done.

liho745 avatar Aug 01 '22 17:08 liho745

@liho745 I have completed the review. I will approve this PR once they are addressed.

tanzeli1982 avatar Aug 02 '22 05:08 tanzeli1982

What is the status of this PR?

rljacob avatar Aug 25 '22 17:08 rljacob

We have detected another bug that is related to MOSART-WM but had not been picked up by the developer tests. I am currently working on it. Hopefully, fix it in the next few days.

liho745 avatar Aug 25 '22 17:08 liho745

@bishtgautam Do you have any remaining comments? If not, could you approve the PR?

liho745 avatar Sep 13 '22 15:09 liho745

Is this ready to be merged?

rljacob avatar Sep 15 '22 17:09 rljacob

Yes.

liho745 avatar Sep 19 '22 04:09 liho745

Telecon: ready but have a higher BGC PR to work on.

rljacob avatar Oct 06 '22 17:10 rljacob

Telecon: ready. Needs conflicts resolved.

rljacob avatar Nov 03 '22 17:11 rljacob

@liho745 This PR is non-BFB as it is adding a new field Flrl_rofmud to the coupler (see https://github.com/E3SM-Project/E3SM/pull/5103/files#diff-ba52cccd5d700657daa5f37d5097103b2a490954e32bcb371eb96919b495e7a3R2157). Should this field be added by default?

bishtgautam avatar Nov 16 '22 17:11 bishtgautam

Is this needed for any BGC v2 simulations?

rljacob avatar Nov 16 '22 17:11 rljacob

@rljacob No, this isn't needed for v2 BGC simulations. Should I revert this PR and try to re-merge this PR after the v2.1 tag is created?

bishtgautam avatar Nov 16 '22 18:11 bishtgautam

Reverting this PR and it will be merged after the v2.1 tag is created.

bishtgautam avatar Nov 16 '22 20:11 bishtgautam

status: being tested locally on next. Need to check restart compatiability.

rljacob avatar Feb 16 '23 18:02 rljacob

status: needs another PR #5454 which updates tests first. Not needed for v3. Integrator has tested and one test is failing for unknown reasons.

rljacob avatar Mar 02 '23 18:03 rljacob

status: needs another PR #5454 which updates tests first. Not needed for v3. Integrator has tested and one test is failing for unknown reasons.

@bishtgautam Are you the integrator? If so, can you walk through the failed tests or let me know how I may help?

liho745 avatar Mar 07 '23 15:03 liho745

The ERS.ne30pg2_r05_EC30to60E2r2.GPMPAS-JRA.compy_intel.mosart-rof_ocn_2way failed related to

MCT::m_AttrVect::indexRA_:: FATAL--attribute not found: "Flrl_inundinf" Traceback

I'm debugging this error today and will keep you updated.

bishtgautam avatar Mar 07 '23 15:03 bishtgautam

I had incorrectly resolved the conflicts in driver-mct/main/prep_rof_mod.F90 that caused the ERS.ne30pg2_r05_EC30to60E2r2.GPMPAS-JRA.compy_intel.mosart-rof_ocn_2way test to fail. After correctly resolving the conflict, the test passes.

bishtgautam avatar Mar 07 '23 19:03 bishtgautam

I had incorrectly resolved the conflicts in driver-mct/main/prep_rof_mod.F90 that caused the ERS.ne30pg2_r05_EC30to60E2r2.GPMPAS-JRA.compy_intel.mosart-rof_ocn_2way test to fail. After correctly resolving the conflict, the test passes.

@bishtgautam Thanks for resolving the test issue. What are the next steps?

liho745 avatar Mar 21 '23 02:03 liho745

@liho745 I merged your branch to next locally on Compy and ran e3sm_land_developer tests. All tests failed and I believe it is because of your recent commit (71da40a). Can you please check to make sure the e3sm_land_developer tests do pass with your branch?

bishtgautam avatar Mar 21 '23 17:03 bishtgautam

Notes: this PR also fixes some fields in the coupler but that should be its own PR. Gautum is testing.

rljacob avatar Apr 06 '23 17:04 rljacob

This PR is replaced by #5591.

bishtgautam avatar Apr 10 '23 22:04 bishtgautam