Repository navigation
Remove non-working aspects of the FLOOD option #80
Description
@mvertens found this issue and does work on this in #74.
The FLOOD option currently can NOT be turned on because of the following logic in RtmMod.F90 where it aborts if it's turned on. But, more than that @mvertens found that it does NOT work if it's turned on. So it's a dead code option.
!-------------------------------------------------------
! Initialize mosart flood - rtmCTL%fthresh and evel
!-------------------------------------------------------
if (do_rtmflood) then
write(iulog,*) subname,' Flood not validated in this version, abort'
call shr_sys_abort(subname//' Flood feature unavailable')
call RtmFloodInit (frivinp_rtm, rtmCTL%begr, rtmCTL%endr, rtmCTL%fthresh, evel)
else
effvel(:) = effvel0 ! downstream velocity (m/s)
rtmCTL%fthresh(:) = abs(spval)
do nt = 1,nt_rtm
do nr = rtmCTL%begr,rtmCTL%endr
evel(nr,nt) = effvel(nt)
enddo
enddo
end ifWe definitely want to remove non-working code, but in further discussion with @swensosc and @slevis-lmwg there is a desire to have the FLOOD capability to still be part of the model, but reworked. I'll file that as a follow on issue to this one.
Activity
mvertens commentedon Jan 21, 2024
@ekluzek - I must admit I'm totally confused about what the current plan is. Is this holding up the PR #74? Since this feature wasn't even working before and the model crashed if you tried to turn on flood - it seems to me that this new capability can be put in subsequent to PR #74 being accepted.
ekluzek commentedon Jan 22, 2024
@mvertens I gave an update here about our plans -- ESCOMP/RTM#43 (comment). @slevis-lmwg and I did meet Friday and we decided exactly what to do, and we will make the tags when we meet again on Monday afternoon. I suppose I should have given an update after Friday since we didn't actually make the tags then.
In our meeting with @swensosc we do need to get it in a state that he can use flooding. He has been using it, although I think even he would admit it's in a non-ideal manner, as he has to add code modes to get it to work. But, he has been using it actively and it's important that he can do this and not lose that functionality now. #81 will be to put this in properly and add testing for it and that will be a future capability that will be done later. So that won't affect anything that is being worked on now...
mvertens commentedon Jan 22, 2024
@ekluzek - thanks for the clarification.
mvertens commentedon Jan 22, 2024
@ekluzek @slevis-lmwg @swensosc - I am wondering why this will not go in after #76 - which is a major cleanup of the code and also provides new needed functionality. It seems that this would be easier to incorporate into the refactored code. Otherwise - this will again need to be redone for the refactored code.
Add back flooding in the code, so @swensosc has a chance of turning i…
mvertens commentedon Jan 22, 2024
On the other hand - if you all feel that this really needs to go in now, I can bring in these changes into my refactored branch. Just let me know what makes the most sense.
wwieder commentedon Jan 22, 2024
Thanks for all the discussion on this. I'm a little unclear on the suggested order of PRs to resolve the following issues, but is this still the plan?