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

Fixing a bug causing model crash by avoiding negative channel storage #6568

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liho745
Copy link
Contributor

@liho745 liho745 commented Aug 28, 2024

MOSART can be forced to crash if negative channel storage values are produced mainly due to round errors, when the channel storage is completely depleted in a single time step. The crashing was reported in several cases of E3SM fully coupled runs, where the kinematic waving routing method was invoked by default. Jon Wolfe was able to reproduce the crashing error in a low-res run and print out detailed information to identify the cause. In this fix, a numerical treatment has been implemented in the kinematic wave routing method, subroutine Routing_KW(), to ensure that any channel storage does not get completely depleted in a single time step. This treatment is physically reasonable as long as MOSART runs at a sub-daily time step. Jon Wolfe has tested this treatment in his low-res coupled run, and it helped avoid crashing.

@rljacob
Copy link
Member

rljacob commented Aug 28, 2024

Is there an issue for this bug?

Copy link
Contributor

@donghuix donghuix left a comment

Choose a reason for hiding this comment

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

I think this modification is not BFB.

else
TRunoff%erout(iunit,nt) = -(TRunoff%erlateral(iunit,nt) + TRunoff%erin(iunit,nt) + TRunoff%wr(iunit,nt)/ theDeltaT)
end if
TRunoff%erout(iunit,nt) = -(TRunoff%erlateral(iunit,nt) + TRunoff%erin(iunit,nt) + TRunoff%wr(iunit,nt)*MaxStorageDepleted/ theDeltaT)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will result in non BFB in the MOSART simulation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. This is non-BFB. It won't affect the overall model simulation (hence science), but it does change the results slightly for the rare cases when a whole channel storage is completely depleted. I added the "non-BFB" label.

Copy link
Member

Choose a reason for hiding this comment

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

Since its potentially non-BFB, this fix would have to go in with 3.1

@liho745 liho745 added the non-BFB PR makes roundoff changes to answers. label Aug 30, 2024
@rljacob rljacob added this to the v3.1beta milestone Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MOSART (river) non-BFB PR makes roundoff changes to answers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants