-
Notifications
You must be signed in to change notification settings - Fork 319
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
Do we need local versions of config flags? #394
Comments
+1 on doing it once with option 4. Not sure if there's a reason for the current method: module-private vars still depend on run-time input namelists: i.e. they're not compile-time constant |
We do not want options 1,2 as they are likely causing some unnecessary performance overhead doing the character matching every time step. We definitely want to set them on init and not keep retrieving them from config. Using local module variables (option 3) keeps most of them encapsulated within the module that's using them. And because they're encapsulated, you can give them shorter variable names with less of a risk of namespace conflict. And for multiple choices, that will keep most changes local to a module if you need to add another option and doesn't clutter up a big comprehensive config module. For variables that need to be accessed by multiple modules, you can either make them shared and include the relevant module where it's used or if that creates complicated dependencies, you could keep them in an independent module (hopefully that's rare). Either way, please convert strings to other forms. Simple binary options should be implemented as logical flags. For conditions where we have multiple options, the preferred way would be to use the Fortran equivalent of enums - basically named integer parameters. So something like this contrived setup: integer myAdvectChoice select case (myAdvectChoice) |
To clarify, when I say convert strings - we can (and should) still have string options in the namelists - just want to convert them during init to something that's more efficient for the integration. |
…o ocean/develop This PR replaces almost all calls to mpas_allocate_scratch_field in the ocean model with straight Fortran allocates. This is done as part of the MPAS framework rewrite, per @philipwjones . This PR also replaces almost all calls to mpas_pool_get_config in subroutines with definitions in ocn_config via use ocn_config. This addresses #394 , and allows #406 to be closed without being merged. There are still a few calls to mpas_allocate_scratch_field that cannot be replaced until more of the MPAS framework rewrite has been completed (namely removal of blocks). This PR only changes the files in the shared subdirectory, not the mode_forward subdirectory.
…develop This PR replaces almost all calls to mpas_allocate_scratch_field in the ocean model with straight Fortran allocates. This is done as part of the MPAS framework rewrite, per @philipwjones . This PR also replaces almost all calls to mpas_pool_get_config in subroutines with definitions in ocn_config via use ocn_config. This addresses #394 , and allows #406 to be closed without being merged. There are still a few calls to mpas_allocate_scratch_field that cannot be replaced until more of the MPAS framework rewrite has been completed (namely removal of blocks). This PR only changes the files in the shared subdirectory, not the mode_forward subdirectory.
In almost every MPAS-Ocean module, config flags are transferred to private module variables. For example, in the ocean core, for del2, we have:
and on init:
Is there any performance reason to do this? It seems like a lot of extra bookkeeping, and extra variables obscure code. Options are:
mpas_pool_get_config
for config flags every time the main routine is called (i.e. every time step)config_*
variables as private module variables and fill them in on inituse ocn_configs
at the top and use config variables by name. Then we would never declare config flags or callmpas_pool_get_config
within these normal modules.I'm asking now because I'm adding new flags for Redi mixing, but it is relevant to all modules. Any of 1-3 we could alter piecemeal. Number 4 (which I prefer) would need a larger coordinated change.
I remember from @philipwjones talk that evaluating strings in if statements is expensive, so there may be two cases here, one for config flags that are strings, and one for flags that are already logicals, integers, or real.
The text was updated successfully, but these errors were encountered: