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

Rework MOSART_MODE and CLM_ACCELERATED_SPINUP check #51

Open
billsacks opened this issue Nov 12, 2021 · 0 comments
Open

Rework MOSART_MODE and CLM_ACCELERATED_SPINUP check #51

billsacks opened this issue Nov 12, 2021 · 0 comments

Comments

@billsacks
Copy link
Member

billsacks commented Nov 12, 2021

Follow-on from #50 , since @ekluzek asked for my comments on how that is done: The general approach taken in #50 seems good (not having one xml variable set another, but instead requiring them to be set explicitly), but I still feel that we should do what I suggested in (2) and (3) in #46 (comment) . To summarize and expand on those comments (note: the numbering below has no relationship to the numbering in that earlier comment):

(1) Since MOSART_MODE is not actually referenced in MOSART itself, it seems like this setting – and the similar settings for other river models – should be moved out of the river models into CMEPS, which is where it's actually needed. The way things are done right now, every river model needs to implement it, and then CMEPS needs logic in a few places where extra clauses will be needed for every river model:

https://github.com/escomp/CMEPS/blob/master/cime_config/buildnml#L88-L91

https://github.com/escomp/CMEPS/blob/master/cime_config/buildnml#L256-L263

https://github.com/escomp/CMEPS/blob/master/cime_config/runseq/driver_config.py#L158-L162

So this setting should be moved out of the river models and into CMEPS, where it can be generically named ROF_MODE. This would both make things less tangled (you wouldn't need to bounce back and forth between CMEPS and the various river models to understand these settings) and more straightforward to users, and for the sake of documentation and tests that change this variable: you wouldn't need to document or change via testmods/usermods different variables depending on the river model: it would always be ROF_MODE. Also, the CMEPS code would be cleaned up, since it could always look at ROF_MODE regardless of which river model is being run.

(Note: I haven't looked carefully to make sure it's possible to do this, especially in RTM.)

(In principle, we could leave this in the river models but still rename it to ROF_MODE, and require that any river model define this xml variable. But I can't see any advantage to that over defining it in CMEPS, since CMEPS is the component that cares about it.)

(2) Once you move this setting into CMEPS and name it generically (ROF_MODE), then the check for compatibility between ROF_MODE and CLM_ACCELERATED_SPINUP should be done in CTSM's build-namelist, since this is really a CTSM-centric check.

(3) I don't love the solution of the blanket "ignore_warnings", and prefer something more specific to a given warning when possible. In this case, I think this can be accomplished by having three options for ROF_MODE: the default would be "ACTIVE"; it could be set to "NULL" to turn off the river model; or it can be set to "ACTIVE_FORCE" (maybe there's a better name for that last one). If you turn on CLM_ACCELERATED_SPINUP, then you will get an error if this is left at the default "ACTIVE" value. "ACTIVE_FORCE" does exactly the same thing as "ACTIVE", except that it also bypasses the consistency check with CLM_ACCELERATED_SPINUP. So when you turn on CLM_ACCELERATED_SPINUP, you also need to change ROF_MODE – either to "NULL" or to "ACTIVE_FORCE".

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

No branches or pull requests

1 participant