How to Review a new component - Revision history

Rhaas: /* Quality criteria */

2022-08-11T13:38:26Z

‎Quality criteria

Rhaas: /* Quality criteria */

2022-08-11T13:38:05Z

‎Quality criteria

Rhaas: /* Quality criteria */

2022-08-10T17:04:21Z

‎Quality criteria

Rhaas: /* Formal criteria */

2022-08-10T17:03:15Z

‎Formal criteria

Rhaas at 16:26, 10 August 2022

2022-08-10T16:26:52Z

Rhaas: /* Quality criteria */

2022-08-02T14:40:22Z

‎Quality criteria

Rhaas: /* Formal criteria */

2022-08-02T13:50:59Z

‎Formal criteria

Rhaas: add draft of review guidelines

2022-08-02T13:48:10Z

add draft of review guidelines

New page

These notes are currently, as of 2022-08-02, a work in progress.

== Overview ==

This document serves as a guideline for reviewers of newly proposed Einstein Toolkit components, it also provides potential contributors with a guideline of what to expect.

Each Einstein Toolkit components has to satisfy a number of criteria for inclusion. These derive from the quality requirements for Einstein Toolkit components laid out in the [https://www.einsteintoolkit.org/contribute.html contributors page], which is authoritative.

The criteria laid out there are:

* Components should be of sufficient quality to be used for peer-reviewed and published science. This includes a basic standard of software engineering, documentation of the software including algorithms and methods, tutorials and examples, and self-tests to demonstrate that the software works correctly.
* To focus development and support, optimize resources, and provide best practises, the Einstein Toolkit components should be of current interest to the community.
* All components must be distributed under an open source licence so that others can use these components without restrictions (except as mandated by scientific integrity), can modify and improve them as necessary, and can pass on these modifications to their collaborators as they see fit.

From these derive guidelines for reviewers.

== Formal criteria ==

These are mostly targeted towards Cactus thorns, but non-thorn contributions should provide equivalent functionality.

* does the contribution have a README file in the [https://bitbucket.org/cactuscode/cactusbase/src/master/Boundary/README Cactus format]? These could be parsed similar to [http://www.cactuscode.org/documentation/Readme_Info.html CactusCode}'s overview page.
** does it list authors of the component (optional but recommended)?
** does it list a maintainer who agrees to look after the component?
** does it list an open source license?
** does it give a brief description of what the component does?
* does the contribution have a documentation.tex file that describes its functionality?
** does it use the Cactus style file and follow the [https://www.einsteintoolkit.org/usersguide/UsersGuide.html#x1-115000C1.8.4 Cactus standards]?
** does it set the "magic markers" <code>% START CACTUS THORNGUIDE</code> and <code>% END CACTUS THORNGUIDE</code> so that it can be included in the online [https://www.einsteintoolkit.org/documentation/ThornGuide.php ThornGuide]
** does the documentation build when running <code>make <THORNNAME>-ThornDoc</code> and <code>make <THORNNAME>-ThornDocHTML</code> (the latter requires [https://packages.debian.org/bookworm/texlive-plain-generic htlatex])
* does it have test cases in its <code>tests</code> directory?
* are there descriptions of all parameters and grid variables in <code>param.ccl</code> and <code>interface.ccl</code>?

== Quality criteria ==

These first two will be handled mostly by the release chair but reviewers are welcome to provide input into these.

* Has there been a publication using the component? This settles the "sufficient quality" criteria
* Has there been public interest in the component? For example voiced in the inclusion ticket. This settles the "current interest" criteria

These will be mostly handled by the reviewers

* Does the code satisfy the criteria laid out in [[How to Review a Patch]]
** is it clear and easy to understand?
** is it sufficiently commented (using English language comments)?
** is there "dead code" or are there large commented out sections?
** are there obvious bugs? Do you notice any corner (or not so corner) cases missing?
* are externally visible symbols, unless in a C++ namespace or Fortran 90 module, prefixed with the thorn name (ie <code>MyThorn_DoSomthing</code> rather than <code>DoSomething</code> (see [https://www.einsteintoolkit.org/usersguide/UsersGuide.html#x1-141000C1.9.7 Cactus Users Guide] for rationale and further details)?
* does the code compile?
* does it run?
* does it (obviously) break anything else already in the Einstein Toolkit?
* do the test cases adhere to [[Adding a test case]]
** do the test cases finish in a reasonable amount of time (less than 10s per test, less than 2min for the whole component)?
** are the test cases "small" (less than 1GB of memory, preferably much less)

== Correctness criteria ==

Correctness is not part of the review, it having been used in peer reviewed publications is taken as sufficient evidence of correctness.

←Older revision		Revision as of 16:26, 10 August 2022
Line 56:		Line 56:

	Correctness is not part of the review, it having been used in peer reviewed publications is taken as sufficient evidence of correctness.		Correctness is not part of the review, it having been used in peer reviewed publications is taken as sufficient evidence of correctness.
		+
		+	== Optional criteria ==
		+
		+	A new component reviewer can suggest that the component provides a gallery example. This will have, at least, two advantages:
		+
		+	# provide a regularly tested (every 6 months), fully worked out example of the code
		+	# increase visibility of new component since it is featured on the Einstein Toolkit home page front page

@@ Line 50: / Line 50: @@
 * does it (obviously) break anything else already in the Einstein Toolkit?
 * do the test cases adhere to [[Adding a test case]]
 ** do the test cases finish in a reasonable amount of time (less than 10s per test, less than 2min for the whole component)?
 ** are the test cases "small" (less than 1GB of memory, preferably much less)
 == Correctness criteria ==

@@ Line 45: / Line 45: @@
 ** is there "dead code" or are there large commented out sections?
 ** are there obvious bugs? Do you notice any corner (or not so corner) cases missing?
-* are externally visible symbols, unless in a C++ namespace or Fortran 90 module in which case the namespace or module name needs to carry the prefix, prefixed with the thorn name (ie <code>MyThorn_DoSomthing</code> rather than <code>DoSomething</code> (see [https://www.einsteintoolkit.org/usersguide/UsersGuide.html#x1-141000C1.9.7 Cactus Users Guide] for rationale and further details)?
+* are externally visible symbols, unless in a C++ namespace or Fortran 90 module in which case the namespace or module name needs to carry the prefix, prefixed with the thorn name? I.e. <code>MyThorn_DoSomthing</code> rather than <code>DoSomething</code>. See [https://www.einsteintoolkit.org/usersguide/UsersGuide.html#x1-141000C1.9.7 Cactus Users Guide] for rationale and further details.
 * does the code compile?
 * does it run?

@@ Line 29: / Line 29: @@
 ** does the documentation build when running <code>make <THORNNAME>-ThornDoc</code> and <code>make <THORNNAME>-ThornDocHTML</code> (the latter requires [https://packages.debian.org/bookworm/texlive-plain-generic htlatex])
 * does it have test cases in its <code>tests</code> directory?
-* are there descriptions of all parameters and grid variables in <code>param.ccl</code> and <code>interface.ccl</code>?
+* are there sensible descriptions of all parameters, scheduled routines and grid variables in <code>param.ccl</code>, <code>schedule.ccl</code> and <code>interface.ccl</code>?
 == Quality criteria ==

@@ Line 19: / Line 19: @@
 These are mostly targeted towards Cactus thorns, but non-thorn contributions should provide equivalent functionality.
-* does the contribution have a README file in the [https://bitbucket.org/cactuscode/cactusbase/src/master/Boundary/README Cactus format]? These could be parsed similar to [http://www.cactuscode.org/documentation/Readme_Info.html CactusCode}'s overview page.
+* does the contribution have a README file in the [https://bitbucket.org/cactuscode/cactusbase/src/master/Boundary/README Cactus format]? These could be parsed similar to [http://www.cactuscode.org/documentation/Readme_Info.html CactusCode]'s overview page.
 ** does it list authors of the component (optional but recommended)?
 ** does it list a maintainer who agrees to look after the component?