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

Add how to add a recipe documentation #3080

Conversation

mo-tgeddes
Copy link
Contributor

@mo-tgeddes mo-tgeddes commented Mar 8, 2023

Description


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

New or updated recipe/diagnostic

New or updated data reformatting script


To help with the number of pull requests:

@mo-tgeddes mo-tgeddes self-assigned this Mar 28, 2023
@mo-tgeddes mo-tgeddes added the Recipe Test Workflow (RTW) Items relevant to the Recipe Test Workflow label Mar 28, 2023
Copy link
Contributor

@alistairsellar alistairsellar left a comment

Choose a reason for hiding this comment

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

Have just noticed that the PR is in draft state, so perhaps too early for a review. Hope the comments are useful in the meantime anyway.

Some refer to wider policy / design questions. Feel free to consider them out of scope if they have already been discussed and decided as part of development.

Final point, when it is ready for review (not draft) please complete the check boxes in the PR description. My understanding of convention for ESMValTool GitHub (according to V at review of my last PR) is that the developer ticks these before assigning for review to show they have considered each aspect.


Can your recipe be added to the RTW?
------------------------------------
The |RTW| will run a recipe and compare the output to a |KGO|.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wider than just this PR, but use of vertical bars with doc/source/common.txt for RTW documentation looks like a divergence from existing conventions in rest of ESMValTool docs. Are we OK with that?

therefore, if the recipe output is not reproducible it cannot be compared to a |KGO|, and the compare task will always fail.

It is recommended that recipes are added to the |RTW| as this will make the process of a release easier,
and should a breaking change be introduced ESMValTool, the author will be notified when the recipe on the |RTW| fails.
Copy link
Contributor

Choose a reason for hiding this comment

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

Author or maintainer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also a wider policy question, but do we plan for maintainers of recipes to be automatically notified by the RTW? Or would the release manager / recent developers be informed first?


To add a recipe to the |RTW|, the first thing that must be done is run the recipe.

Take note of the resources required to run the recipe and the time it takes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the audience for this include developers of new recipes? Or will it more generally be an SSE? Some scientists will not know how to do this, particularly if the recipe is cheap enough to run on VDI. Does it need some tips?

* slow - Anything above this.


* Open the flow.cylc file and edit the task parameters of the group the added recipe belongs to::
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, code design question rather than a documentation comment... these lists could get very long. It would feel more natural to me to group parameters by task, like

[radiation_budget]
    resource_group = fast

But I've not got visibility of the whole workflow design, so if you have already discussed this and rejected it then I'm happy to trust your collective judgement.

fast = radiation_budget, <your_recipe_name>
medium = ensclus, heatwaves_coldwaves

The name of the task parameter must match the file name of the recipe to be added
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The name of the task parameter must match the file name of the recipe to be added
The name of the task parameter must match the file name of the recipe to be added, minus the `.yml` extension.


The name of the task parameter must match the file name of the recipe to be added

* Run the branch of |RTW| on all sites and ensure the added recipe passes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, does the intended audience know how to do this. Which are "all sites" for now, and does any user have access to the all? Once Levante (DKRZ) and NCI (Australia) included in RTW sites, there will be no user who has access to them all (though I think that GitHub esmvalbot runs the recipe on Levante when asked).

@ehogan
Copy link
Contributor

ehogan commented Jun 25, 2024

Superseded by #3614 😊

@ehogan ehogan closed this Jun 25, 2024
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 this pull request may close these issues.

3 participants