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

Use site-specific lists of recipes in the RTW #3724

Open
ehogan opened this issue Jul 19, 2024 · 6 comments · May be fixed by #3856
Open

Use site-specific lists of recipes in the RTW #3724

ehogan opened this issue Jul 19, 2024 · 6 comments · May be fixed by #3856
Assignees
Labels
Recipe Test Workflow (RTW) Items relevant to the Recipe Test Workflow
Milestone

Comments

@ehogan
Copy link
Contributor

ehogan commented Jul 19, 2024

I realised as I was adding recipes to run on DKRZ (via #3708) that those recipes would not be able to run at other sites, since the resources wouldn't be defined (so the recipes would likely run out of time and / or memory, using the default resources).

After a quick discussion with @alistairsellar, we agreed to use site-specific lists of recipes (@alistairsellar mentioned that the RTW running on JASMIN began to fail due to recent additions of recipes that were introduced at the MO).

@ehogan ehogan added the Recipe Test Workflow (RTW) Items relevant to the Recipe Test Workflow label Jul 19, 2024
@ehogan ehogan added this to the v2.12.0 milestone Jul 19, 2024
@ehogan ehogan self-assigned this Jul 19, 2024
chrisbillowsMO added a commit that referenced this issue Dec 17, 2024
@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Dec 17, 2024

If you exclude a recipe that has custom directives, you get this quite vague error:

ParamExpandError: parameter fast out of range: fast=recipe_ocean_multimap

@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Dec 20, 2024

I used {% from 'inc/' ~ SITE ~ '-recipes.cylc' import FAST_RECIPES, MEDIUM_RECIPES %} because the more comprehensive {% include 'inc/' ~ SITE ~ '-recipes.cylc' %} wasn't working.

The Jinja2 docs suggest {% include 'inc/' ~ SITE ~ '-recipes.cylc' with context %} should resolve the problem - but it didn't.

I didn't try the Cylc docs namespace solution - it seemed more complex than the import solution.

@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Dec 20, 2024

  • Q: I used is defined rather than != "" to avoid setting FAST/MEDIUM_RECIPES in the rose-suite.conf - but perhaps better that they are done the same way as SITE (or SITE is changed to work with is_defined too?).

Confirmed fine as is.

@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Dec 20, 2024

  • 6c2f04b committed this for discussion only - it's way too complex?

Probably living with the standard error would be preferable:

  • ParamExpandError: parameter fast out of range: fast=recipe_ocean_amoc does give a clue...
  • Search for fast=recipe_ocean_amoc you'd find the problem location... though not the problem itself

Alternative solutions

  • Definitely sure there is no CYLC_PARAMETERS_all we can iterate over?
  • Capture the directives in the site inc page. E.g.
        {
            'recipe' :  'recipe_autoassess_landsurface_soilmoisture',
            'max_time' : 'PT2M',
            'max_memory' : '3G',
        },

@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Dec 23, 2024

In 50fe052, per discussion with @alistairsellar - completed/tidied up moving the custom directives out of the site.cylc file and into the site-recipes.cylc.

Documentation to be updated, pending wider discussion and tweaks with @ehogan after Xmas.

chrisbillowsMO added a commit that referenced this issue Dec 23, 2024
@chrisbillowsMO
Copy link
Contributor

chrisbillowsMO commented Jan 8, 2025

Documenting discussion with @ehogan

  • Agreed with having the recipes list and their directives into site-recipes (splitting these details across two files, and generating a vague error if you delete the recipe is worth avoiding)

Additional changes

  • Move out of inc and into site / retain the name site-recipes.cylc
    • opt has to be seperate for the CLI args to function; everything else is a site config so site makes sense
  • Remove the default settings options / include the details for every recipe
  • The fast/medium parameterisation is now generic / move out of site.cylc and into flow.cylc
  • Update metoffice-recipes docstring
  • Update docs / adding recipe process

chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
chrisbillowsMO added a commit that referenced this issue Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Recipe Test Workflow (RTW) Items relevant to the Recipe Test Workflow
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants