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

Remove ability to use different routing methods #102

Closed
ekluzek opened this issue Jun 19, 2024 · 4 comments · Fixed by #94
Closed

Remove ability to use different routing methods #102

ekluzek opened this issue Jun 19, 2024 · 4 comments · Fixed by #94

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Jun 19, 2024

There's some code fragments that aren't exercised to run with different RoutingMethod options. It's hardcoded to only one (1), so we might as well remove the fragments that aren't testing for other options.

Looking for it in the code:

src/riverroute/MOSART_physics_mod.F90:      if(Tctl%RoutingMethod == 1) then
src/riverroute/MOSART_physics_mod.F90:      else if(Tctl%RoutingMethod == 2) then
src/riverroute/MOSART_physics_mod.F90:      else if(Tctl%RoutingMethod == 3) then
src/riverroute/MOSART_physics_mod.F90:      else if(Tctl%RoutingMethod == 4) then
src/riverroute/RtmMod.F90:      Tctl%RoutingMethod = 1
src/riverroute/RunoffMod.F90:     integer  :: RoutingMethod    ! Flag for routing methods. 1 --> variable storage method from SWAT model; 2 --> Muskingum method?

We could leave the variable in place just in case it's added back again by a hydrologist. But, as the other methods aren't tested it makes sense to remove the options and update the comments that talk about it.

@swensosc @slevis-lmwg

@mvertens
Copy link
Contributor

@ekluzek - I disagree that these variables should be kept in place. MOZART physics is not going to be developed further and these variables have not been exercised in years. Its confusing to have logic for code that is not planned to be developed.

@ekluzek
Copy link
Contributor Author

ekluzek commented Jun 20, 2024

The subject of this issue is the removal of these options as you did in #94. It's not about keeping them around.

@wwieder
Copy link

wwieder commented Jun 20, 2024

Hey, I sense some tension here that I wonder if we could address with more direct communication?

Separately, I think it's also worth noting that on the CESM side we want a functional implementation of MOSART that enables glacier to ocean communication with evolving ice sheets, which it seems like is accomplished in #94, and (nearly) ready for @slevis-lmwg to merge?

Beyond CESM3, however, we're aiming to move to mizuRoute. As such, I don't know that we need to create perfect MOSART code, just functional for CESM3. I'll defer to the SE team to settle on what this looks like.

@mvertens
Copy link
Contributor

@ekluzek - sorry for misunderstanding this issue. I had it backwards.
@wwieder - I have been working with both the CESM and NorESM land ice groups to validate this implementation and I believe everyone has signed off on it.

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

Successfully merging a pull request may close this issue.

3 participants