add support for user_mods in compset definition #1347
Conversation
|
Thanks, @jedwards4b , for implementing this. I have two thoughts: First, I feel it's important that, if a user_mods is included in a compset, then that information should be printed to stdout when creating a case, and also echoed to the README.case file where (IIRC) the compset information is given. The reason is: If someone wants to create a one-off user-compset based on one of these compsets, then it's important for them to know that this compset includes a user_mods directory. Second: I think some code mods are needed in user_mod_support.py in order to be consistent about which mods take precedence: the compset's mods or the mods specified on the command line. Personally, I feel that the mods specified on the command line should take precedence, because the user explicitly asked for those. However, the most important thing is consistency between user_nl files, shell_commands and SourceMods in terms of which takes precedence. We ( @jedwards4b @mnlevy1981 and myself) spent some time making sure user_mod_support consistently enforces the rule that a given user_mods directory takes precedence over any mods that it includes. However, I don't think we considered the possibility that apply_user_mods may be called twice. If I understand the code correctly:
However, if we implemented the changes suggested by this comment: # it may be desireable to reverse include_dirs above the
# previous loop and then append user_nl changes rather than prepend themthen that would change the precedence of user_nl files. I would suggest the following:
for user_mods in (user_mods_dir, self._user_mods):so that we apply the command line second. (However, I don't feel strongly about this last point.) With these changes, I believe we would maintain the current behavior that a given user_mods directory takes precedence over any mods that it includes, and we would have the consistent rule that the second call to apply_user_mods takes precedence over the first - in terms of user_nl files, shell_commands and SourceMods. However, we should test (both for include dirs and for double-application of apply_user_mods) to make sure my understanding is correct. |
Now --user-mods on the command line (including testmods) will take
precedence over the user_mods set by the compset - for user_nl files,
shell_commands and SourceMods.
I have tested this with this diff to the A compset
diff --git a/src/drivers/mct/cime_config/config_compsets.xml b/src/drivers/mct/cime_config/config_compsets.xml
index c11354e..7e6c2c9 100644
--- a/src/drivers/mct/cime_config/config_compsets.xml
+++ b/src/drivers/mct/cime_config/config_compsets.xml
@@ -40,6 +40,7 @@
<compset>
<alias>A</alias>
<lname>2000_DATM%NYF_SLND_DICE%SSMI_DOCN%DOM_DROF%NYF_SGLC_SWAV</lname>
+ <user_mods>/Users/sacks/temporary/user_mods_compset</user_mods>
</compset>
<compset>
Along with this create_newcase command:
./create_newcase -case test_0414m -compset A -res f45_g37 \
--run-unsupported \
--user-mods-dir /Users/sacks/temporary/user_mods_command_line
where the contents of the two relevant user_mods directories are:
--- user_mods_compset/shell_commands ---
./xmlchange STOP_N=101
--- user_mods_compset/SourceMods/src.drv/mysrc.F90 ---
user_mods_compset
--- user_mods_compset/user_nl_cpl ---
user_mods_compset
--- user_mods_command_line/shell_commands ---
./xmlchange STOP_N=102
--- user_mods_command_line/SourceMods/src.drv/mysrc.F90 ---
user_mods_command_line
--- user_mods_command_line/user_nl_cpl ---
user_mods_command_line
The final contents are:
--- user_nl_cpl ---
user_mods_compset
user_mods_command_line
--- shell_commands ---
./xmlchange --force STOP_N=102
--- SourceMods/src.drv/mysrc.F90 ---
user_mods_command_line
And
$ ./xmlquery STOP_N
STOP_N: 102
thus demonstrating that the user_mods on the command-line takes
precedence over the compset's user_mods.
|
@jedwards4b and @mvertens - I have added a commit where I think I have this working right in terms of precedence. This also fixes the problems that Mariana and I ran into (at least, I think it does: Mariana alluded to some other problem that I'm unclear about). I still want to do a bit more testing; I'll probably set up unit tests for apply_user_mods to make sure it's handling various recursive includes correctly. But I have at least tested the primary use case this PR is targeting. |
|
Okay, I have finished addressing and testing the second point in my big comment. Someone should still address this one:
And, as @mvertens pointed out, my changes need a get_resolved_value (?) somewhere. |
|
Superceeded by #1366 |
#1347 Optimize physics load balancing twin algorithm initialization When load balancing physics computation on a latlon mesh, there is an advantage to assigning columns to chunks in pairs, pairing each column with its 'twin' in the other hemisphere, latitude reflected around the equator, and offset 180 degrees in longitude. While a similar approach should be advantageous, or at least no worse than no doing it, with a non-latlon mesh, the cost of initialization of this 'twin' algorithm was too high for the HOMME spectral element mesh to be acceptable even though only part of the initialization cost. Because of this, the twin algorithm is disabled by default when using a non-latlon mesh. (For a latlon mesh, the twin algorithm is enabled by default.) Note that a runtime parameter can be used to override the default. A simple change to the initialization algortihm drops the cost to an acceptable level, less than 2 minutes even on one core of Mira and for an ne120 HOMME mesh. In performance experiments, using the twin algorithm has not improved the load balance compared to not using it for current ACME production-like cases, and the default disabling the twin algorithm is not being changed. However, this optimization to the initialization algorithm will make it feasible to re-examine this option in the future. This change will not be tested by the ACME test suites, as the twin algorithm is disabled, and it is also irrelevant unless atmospheric physics load balancing is enabled. Experiments with F compsets and the CAM-SE dycore for both ne30 and ne120 meshes have been conducted, indicating that the same load balancing redistribution of columns is computed by the twin algorithm with and without this optimization. Tests with FV and Spectral Eulerian dycores have not been conducted. [BFB]
Allows for the specification of user_mods in the compset definition. If both compset definition and
command line argument (--user-mods-dir) are specified then both are applied with command line applied first and compset second.
Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes [CIME Github issue #]
User interface changes?:
Code review: