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

Configure UM via models #139

Merged
merged 17 commits into from
Sep 20, 2024
Merged

Configure UM via models #139

merged 17 commits into from
Sep 20, 2024

Conversation

penguian
Copy link
Contributor

@penguian penguian commented Aug 12, 2024

Closes #135

The changes are extensive.

  1. The first main change is the introduction of the model variant, and a corresponding model/ directory. The string value of model determines which subdirectory of model/ to use to find the corresponding rose-app.conf file. There are currently 3 subdirectories, vn13 (default), vn13p0-rns (corresponding to the RNS suite u-dg768) and vn13p5-rns.
  2. The second main change is that almost every environment variable in model/*/rose-app.conf now has a corresponding variant.
    1. The environment variables in rose-app.conf that take the values true or false correspond to either bool or off/on variants.
      1. The bool variants are normal Spack bool variants, taking the value False or True. The lambda functionbool_to_str transforms these to the environment variable values "false" or "true", determined by the value of the variant. The value given in rose-app.conf is ignored. Currently only eccodes and netcdf are bool variants. The default value of thesebool variables is True, corresponding to the value used in the RNS suite u-dg768.
      2. The off/on variants take the values "none", "off" or "on". The Spack default is "none". Leaving the value as "none" results in the corresponding environment variable being set to the value given in rose-app.conf. Otherwise, the lambda function off_on_to_str transforms "off" to "false" or "on" to "true" and this is the value used for the corresponding environment variable, overriding the value given in rose-app.conf.
    2. The component revision environment variables in rose-app.conf, other than um_rev, correspond to revision variants. For revision variants, the Spack default is "none". If the variant value left as "none", then if the value in rose-app.conf defined and non-empty, it is used as the environment variable value, otherwise the value is "um{version}" where version is the UM version. If the variant value is not "none" then it is used as the environment variable value, overriding any value in rose-app.conf.
    3. All other environment variables in rose-app.conf, with the exception of "um_rev", correspond to str variants. The default value for each of these variants is "none". Leaving this value as "none" results in the corresponding environment variable being set to the value given in rose-app.conf. If the variant is set to a value other than "none", then this value overrides the value given in rose-app.conf. This effectively makes the value in rose-app.conf the default.

@penguian penguian self-assigned this Aug 12, 2024
@penguian penguian linked an issue Aug 12, 2024 that may be closed by this pull request
@penguian penguian marked this pull request as draft August 12, 2024 06:15
@penguian penguian requested a review from harshula August 13, 2024 08:58
@penguian penguian marked this pull request as ready for review August 13, 2024 08:59
@penguian penguian marked this pull request as draft August 14, 2024 01:16
@penguian penguian removed the request for review from harshula August 14, 2024 01:31
@penguian penguian requested a review from harshula August 19, 2024 01:33
@penguian penguian marked this pull request as ready for review August 19, 2024 01:33
Copy link
Collaborator

@harshula harshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed in VC.

  • Put the defaults for conf variables in the *.conf files.
  • Do not rename conf variables when used for variant names.
  • Treat boolean variants as string variants by using strings "on"/"off"
  • Add TODO comments where there is a good opportunity improve clarity by creating functions that can be called from multiple locations.
  • Don't include DR_HOOK as a variant if it's not usable.

@penguian
Copy link
Contributor Author

All comments addressed, except that eccodes and netcdf are boolean variants so that dependencies can be specified. These both default to True.

@penguian penguian requested a review from harshula August 21, 2024 06:39
@penguian penguian marked this pull request as draft August 22, 2024 01:18
@penguian penguian marked this pull request as ready for review August 22, 2024 03:18
packages/um/package.py Outdated Show resolved Hide resolved
harshula
harshula previously approved these changes Aug 22, 2024
Copy link
Collaborator

@harshula harshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, well done!

@penguian penguian merged commit 3e1b31f into main Sep 20, 2024
1 check passed
@penguian penguian deleted the 135-configure-um-via-models branch September 20, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Configure UM via models
2 participants