You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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".
The text was updated successfully, but these errors were encountered:
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 beROF_MODE
. Also, the CMEPS code would be cleaned up, since it could always look atROF_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 betweenROF_MODE
andCLM_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 onCLM_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 withCLM_ACCELERATED_SPINUP
. So when you turn onCLM_ACCELERATED_SPINUP
, you also need to changeROF_MODE
– either to "NULL" or to "ACTIVE_FORCE".The text was updated successfully, but these errors were encountered: