From 9f10004fd9bf66b876a1fceed520cfeedfc781eb Mon Sep 17 00:00:00 2001 From: mn3981 Date: Wed, 16 Oct 2024 10:15:54 +0100 Subject: [PATCH 1/4] Update pull request template and CONTRIBUTING.md - Update pull_request_template.md to include following PROCESS style guide - Update CONTRIBUTING.md to ensure implementation follows PROCESS style guide for names of variables and functions Update development standards documentation - Add information about Python PEP8 style guide and its compatibility with Fortran90 - Add guidelines for line length and double declarations - Add guidelines for naming conventions of functions, switches, constants, and variables - Add guidelines for length, physical type, coordinates and dimensions, and units - Add guidelines for docstrings and code documentation using FORD Update mkdocs.yml - Update navigation structure by renaming "Reference" section to "Models" and reorganizing sub-sections --- .github/pull_request_template.md | 1 + CONTRIBUTING.md | 5 +- .../proc-pages/development/standards.md | 70 +++++++++++++++++-- mkdocs.yml | 2 +- 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d182944d99..b8322ce3e9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,6 +6,7 @@ I confirm that I have completed the following checks: +- [ ] My changes follow the [PROCESS style guide](https://ukaea.github.io/PROCESS/development/standards/) - [ ] I have justified any large differences in the regression tests caused by this pull request in the comments. - [ ] I have added new tests where appropriate for the changes I have made. - [ ] If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cb00434012..2e083d5ee3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,7 @@ Please discuss any feature ideas you have with the developers before submitting * Submit an issue (if one does not exist for this feature/ bug) that documents the proposed change. * Fork our repository. * Create a branch off `main` with an appropriate name (e.g. `feature-abc`). -* Make the relevant changes for the repository (ensuring the changes do not creep away from the scope of the issue). +* Make the relevant changes for the repository (ensuring the changes do not creep away from the scope of the issue). Also ensure that the implementation follows the PROCESS [style guide](https://ukaea.github.io/PROCESS/development/standards/) for names of variables and functions etc. * Discuss any problems or development choices in the issue and keep everyone updated on the progress of your development. * If the changes are notable and it would benefit other users to be aware, [create a changelog entry](https://ukaea.github.io/PROCESS/development/versioning/). * Finally, submit a pull request onto the `main` branch: @@ -35,4 +35,5 @@ Please remember that all contributions are made under the [MIT license](https:// ### Testing PROCESS has unit, integration, and regression tests. Any new functionality must be appropriately tested. Sometimes, changes may require other tests to be changed. These changes should be justified in the pull request description. Tests can be run locally by following [our testing documentation](https://ukaea.github.io/PROCESS/development/testing/). All pull requests will also be run against our GitHub actions which will run all of the tests and report back to the reviewer any failures. **The unit and integration tests must pass on the CI for the changes to be accepted**. -Regression tests, due to the nature of PROCESS, can change as model changes affect the optima which PROCESS converges to. A reviewer will review these changes to ensure they are minor and justified. We recommend justifying how a regression test is changing in the pull request discussion, a reviewer will likely request this anyway. For convenience, the CI system runs a 0% tolerance job that will highlight all differences between the current version of PROCESS on the `main` branch and your modified version of PROCESS; the 5% job excludes all differences under 5% differences between the two versions. \ No newline at end of file +Regression tests, due to the nature of PROCESS, can change as model changes affect the optima which PROCESS converges to. A reviewer will review these changes to ensure they are minor and justified. We recommend justifying how a regression test is changing in the pull request discussion, a reviewer will likely request this anyway. For convenience, the CI system runs a 0% tolerance job that will highlight all differences between the current version of PROCESS on the `main` branch and your modified version of PROCESS; the 5% job excludes all differences under 5% differences between the two versions. + diff --git a/documentation/proc-pages/development/standards.md b/documentation/proc-pages/development/standards.md index d2de731716..4a23b61ea6 100644 --- a/documentation/proc-pages/development/standards.md +++ b/documentation/proc-pages/development/standards.md @@ -1,11 +1,17 @@ # Style Guide +`PROCESS` follows the Python [PEP8](https://peps.python.org/pep-0008/) style guide for its layout and namespace style. This should not conflict with Fortran90 in terms of names but may with the line lengths. + +-------------------- + ## Line Length For optimal readability, a limit of 100 characters for maximum line length has been set. This is below the maximum line length of 132 characters for Fortran (to prevent compilation errors) and prevents long lines that run on past the edge of the screen wasting programmers time with scrolling. +-------------------- + ## Double declarations PROCESS uses the Fortran 2008+ intrinsic precision module as shown in the example below. The @@ -20,16 +26,41 @@ real(dp) :: b ``` +all new models should have their own function + +-------------------- + ## Naming conventions +!!! quote "The Zen of Python" + + *“Explicit is better than implicit.”* + + *"Readability counts."* + +### Functions + +Use a lowercase word or words. Separate words by underscores to improve readability. + +### Switches + +Switches should start with the `i_` prefix in their name, be of integer type and should be indexed from 0. +Switches should by their nature not override other switches. -### Case +### Constants -All variables should be lower case. +Use an uppercase single letter, word, or words. Separate words with underscores to improve readability. + +Refrain from declaring or typing known numerical constants directly in the code. Instead call the value from `constants.f90` +If the constants doesn't exist then add it with a source link and uncertainty value. + +### Variables + +Use a lowercase single letter, word, or words. Separate words with underscores to improve readability. ### Length -Try to keep variable names to a sensible length. Abbreviations of some parts of the name are suitable e.g. div for divertor. Use underscores to separate words. +Try to keep names to a sensible length while also keeping the name explicit and descriptive. ### Physical Type @@ -51,10 +82,10 @@ Inside PROCESS all variables should be in SI units unless otherwise stated. For ```fortran ! Fusion power [W] -p_fusion = 1000.0d6 +fusion_power = 1000.0d6 ! Fusion power [MW] -p_fusion_mw = 1000.0d0 +fusion_power_MW = 1000.0d0 ``` ### Coordinates and dimensions @@ -119,7 +150,32 @@ ii Please see issue 940 to discuss new conventions. -# Code Documentation Using FORD +## Docstrings + +```python +def function_name(param1, param2): + """ + Brief description of what the function does. + + Args: + param1 (type): Description of the first parameter with units. + param2 (type): Description of the second parameter with units. + + Returns: + return_type: Description of the return value with units. + + Notes: + Detailed description of the function. This can include information about the algorithm, + any important notes, and other relevant details. + + References: + - Reference 1 + - Reference 2 + (Use a verbose reference style that has a clickable DOI or HTML link.) + """ +``` + +## Code Documentation Using FORD PROCESS uses FORD (FORtran Documentation) to automatically generate documentation from comments in the FORTRAN code. FORD parses FORTRAN source to understand the structure of the project as well @@ -161,7 +217,7 @@ use "!!" for consistency. The rationale behind this and further information is i The FORD project on github can be found [here](https://github.com/Fortran-FOSS-Programmers/ford). -## Example of FORD documentation for a subroutine (constraint equation) +### Example of FORD documentation for a subroutine (constraint equation) ```fortran diff --git a/mkdocs.yml b/mkdocs.yml index 5658112ef1..4ddb8a4395 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -38,7 +38,7 @@ nav: - Standards: development/standards.md - F2Py Standards: development/f2py-standards.md # - Versioning: development/versioning.md - - Reference: + - Models: - Physics Models: - Plasma: - Overview: physics-models/plasma_overview.md From c9192776f0368054e712379cdf3a4d9361350e45 Mon Sep 17 00:00:00 2001 From: mn3981 Date: Tue, 22 Oct 2024 11:18:41 +0100 Subject: [PATCH 2/4] Refactor variable naming conventions and add type hints - Updated the standards.md file to clarify the naming conventions for variables, including the use of underscores and capital letters for unit differentiations. - Added a prefix "f_" for variables representing fractions. - Included a reminder to use SI units unless otherwise stated. - Added a section on type hints and referenced PEP-484 for guidance. - Updated the docstring templates for functions and classes. - Added guidelines for comments. Fixes #940 --- .../proc-pages/development/standards.md | 70 ++++++++++++++++--- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/documentation/proc-pages/development/standards.md b/documentation/proc-pages/development/standards.md index 4a23b61ea6..1ae08ea262 100644 --- a/documentation/proc-pages/development/standards.md +++ b/documentation/proc-pages/development/standards.md @@ -40,7 +40,7 @@ all new models should have their own function ### Functions -Use a lowercase word or words. Separate words by underscores to improve readability. +Use a lowercase word or words. Separate words by underscores(`_`) to improve readability. ### Switches @@ -57,6 +57,11 @@ If the constants doesn't exist then add it with a source link and uncertainty va ### Variables Use a lowercase single letter, word, or words. Separate words with underscores to improve readability. +If converting between units it may be required to have some capital letters at the end of the variable name to differentiate between different orders of magnitude, `m` and `M` etc. + +#### Variables representing fractions + +If a variable is intended to demonstrate a fraction of a value or distribution etc. Then it should start with the `f_` prefix. ### Length @@ -76,6 +81,8 @@ Another example would be pulse length time_pulse_length = 7200.0 ``` +------------------ + ### Units Inside PROCESS all variables should be in SI units unless otherwise stated. For example: @@ -125,33 +132,41 @@ ii | Variable name | Description | Units | | ------------- | ----------- | :---: | -| `i_plasma` | Plasma current | A | -| `i_plasma_ma` | Plasma current | MA | +| `plasma_current` | Plasma current | A | +| `plasma_current_MA` | Plasma current | MA | | `b_t_onaxis` | Toroidal field on-axis | T | | `b_t_max` | Max toroidal field | T | | `n_electron_vol` | Volume average electron density | m-3 | | `t_electron_vol_ev` | Volume avgerage electron temperature | eV | -| `m_steel` | Mass of steel | kg | -| `m_steel_tonne` | Mass of steel | tonne | +| `mass_steel` | Mass of steel | kg | +| `mass_steel_tonne` | Mass of steel | tonne | | `e_neutron_ev` | Energy of neutron | eV | -| `e_neutron_mev` | Energy of neutron | MeV | +| `e_neutron_MeV` | Energy of neutron | MeV | | `v_tf_dump` | TF dump voltage | V | | `time_plant_life` | Plant lifetime | s | | `time_plant_life_yrs` | Plant lifetime | years | | `dr_tf_inboard_leg` | TF coil inboard leg radial thickness | m | | `dr_blanket_inboard` | Inboard blanket thickness | m | | `velocity_coolant` | TF centrepost coolant velocity | m/s | -| `vol_plasma` | Plasma volume | m3 | -| `a_plasma` | Plasma area | m2 | +| `plasma_volume` | Plasma volume | m3 | +| `plasma_area` | Plasma area | m2 | | `angle_div_target` | Divertor target angle | radians | | `angle_div_target_deg` | Divertor target angle | deg | | `sig_tf_r` | TF radial stress | Pa | | `` | | | -Please see issue 940 to discuss new conventions. +Please see issue [#940](https://github.com/ukaea/PROCESS/issues/940) to discuss new conventions. + +## Type-Hints + +It is greatly encouraged and recommended to include type hints for all inputs and outputs in Python. Please follow the guidelines set out in [PEP-484](https://peps.python.org/pep-0484/). ## Docstrings +### Functions + +If writing in new Python functions please use the docstring template below. + ```python def function_name(param1, param2): """ @@ -175,6 +190,43 @@ def function_name(param1, param2): """ ``` +### Classes + +If writing in new Python classes please use the docstring template below. + +```python +class ClassName: + """ + Brief description of the class. + + Detailed description of the class. This can include information about the purpose + of the class, how it should be used, and any other relevant details. + + Attributes: + attribute1 (type): Description of attribute1. + attribute2 (type): Description of attribute2. + attribute3 (type): Description of attribute3. + attribute4 (type): Description of attribute4. + + Methods: + method1(): Description of method1. + method2(): Description of method2. + method3(): Description of method3. + method4(): Description of method4. + """ +``` + +## Comments + +- **Comments that contradict the code are worse than no comments** + +- Comments should be complete sentences. The first word should be capitalized, unless it is an identifier that begins with a lower case letter. + +- Use inline comments sparingly. + +- Comments above apply to code below. + + ## Code Documentation Using FORD PROCESS uses FORD (FORtran Documentation) to automatically generate documentation from comments From c8aec2f6c0e4858c5ccd480d0c1709ef48cc3d0a Mon Sep 17 00:00:00 2001 From: mn3981 Date: Fri, 25 Oct 2024 15:27:43 +0100 Subject: [PATCH 3/4] Update coding standards and documentation guidelines - Adjusted the maximum line length to 79 characters, following PEP8 recommendations, to improve readability and prevent scrolling. - Updated naming conventions for switches and constants. - Added guidelines for variable names related to fractions and constraint equations. - Provided documentation style guidelines, including the use of Google-style docstrings and recommended VSCode extensions. --- .../proc-pages/development/standards.md | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/documentation/proc-pages/development/standards.md b/documentation/proc-pages/development/standards.md index 1ae08ea262..45d629e56e 100644 --- a/documentation/proc-pages/development/standards.md +++ b/documentation/proc-pages/development/standards.md @@ -6,9 +6,7 @@ ## Line Length -For optimal readability, a limit of 100 characters for maximum line length has been set. This is -below the maximum line length of 132 characters for Fortran (to prevent compilation errors) and -prevents long lines that run on past the edge of the screen wasting programmers time with scrolling. +For optimal readability, a limit of 79 characters for maximum line length has been encouraged, as recommended in [PEP8](https://peps.python.org/pep-0008/). This is below the maximum line length of 132 characters for Fortran (to prevent compilation errors) and prevents long lines that run on past the edge of the screen wasting programmers time with scrolling. -------------------- @@ -45,7 +43,6 @@ Use a lowercase word or words. Separate words by underscores(`_`) to improve rea ### Switches Switches should start with the `i_` prefix in their name, be of integer type and should be indexed from 0. -Switches should by their nature not override other switches. ### Constants @@ -63,6 +60,10 @@ If converting between units it may be required to have some capital letters at t If a variable is intended to demonstrate a fraction of a value or distribution etc. Then it should start with the `f_` prefix. +#### F-values + +Variables used within constraint equations to scale iteration variables (f-values) should start with the `f` prefix without an underscore before the next word. + ### Length Try to keep names to a sensible length while also keeping the name explicit and descriptive. @@ -163,6 +164,20 @@ It is greatly encouraged and recommended to include type hints for all inputs an ## Docstrings +The docstring style is that of the [Google type](https://google.github.io/styleguide/pyguide.html#s3.8.1-comments-in-doc-strings). Though there are some additions for `Notes` and `References` in order to give mathematical reasoning and sources to some functions. + +The following VSCode extensions can be used to generate the bulk of the docstring in the Google style: + +------------------------------------ + +Name: autoDocstring - Python Docstring Generator + +Id: njpwerner.autodocstring + +VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=njpwerner.autodocstring + +---------------------------------- + ### Functions If writing in new Python functions please use the docstring template below. From 44ec8c2c095b61d8829f49f59b50f3e0d599f17e Mon Sep 17 00:00:00 2001 From: mn3981 Date: Wed, 30 Oct 2024 14:37:20 +0000 Subject: [PATCH 4/4] Update documentation to reflect Sphinx-style docstrings and enhance clarity --- .../proc-pages/development/standards.md | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/documentation/proc-pages/development/standards.md b/documentation/proc-pages/development/standards.md index 45d629e56e..5cbad1f700 100644 --- a/documentation/proc-pages/development/standards.md +++ b/documentation/proc-pages/development/standards.md @@ -164,19 +164,7 @@ It is greatly encouraged and recommended to include type hints for all inputs an ## Docstrings -The docstring style is that of the [Google type](https://google.github.io/styleguide/pyguide.html#s3.8.1-comments-in-doc-strings). Though there are some additions for `Notes` and `References` in order to give mathematical reasoning and sources to some functions. - -The following VSCode extensions can be used to generate the bulk of the docstring in the Google style: - ------------------------------------- - -Name: autoDocstring - Python Docstring Generator - -Id: njpwerner.autodocstring - -VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=njpwerner.autodocstring - ----------------------------------- +The docstring style is that of the [Sphinx type](https://www.sphinx-doc.org/en/master/index.html). Though there are some additions for `Notes` and `References` in order to give mathematical reasoning and sources to some functions. ### Functions @@ -187,22 +175,23 @@ def function_name(param1, param2): """ Brief description of what the function does. - Args: - param1 (type): Description of the first parameter with units. - param2 (type): Description of the second parameter with units. + Detailed description of the function. This can include information about the algorithm, + any important notes, and other relevant details. - Returns: - return_type: Description of the return value with units. + :param type param1: Description of the first parameter. + :param type param2: Description of the second parameter. + :returns: Description of the return value. + :rtype: return_type + :raises ExceptionType: Description of the exception raised (if any). - Notes: - Detailed description of the function. This can include information about the algorithm, - any important notes, and other relevant details. + :notes: + - Additional notes about the function. + - Any important considerations or caveats. - References: - - Reference 1 - - Reference 2 - (Use a verbose reference style that has a clickable DOI or HTML link.) - """ + :references: + - Reference 1: Description of the reference. + - Reference 2: Description of the reference. + """ ``` ### Classes @@ -210,7 +199,7 @@ def function_name(param1, param2): If writing in new Python classes please use the docstring template below. ```python -class ClassName: +class ExampleClass: """ Brief description of the class. @@ -221,14 +210,23 @@ class ClassName: attribute1 (type): Description of attribute1. attribute2 (type): Description of attribute2. attribute3 (type): Description of attribute3. - attribute4 (type): Description of attribute4. Methods: - method1(): Description of method1. - method2(): Description of method2. - method3(): Description of method3. - method4(): Description of method4. + method1(param1, param2): Description of method1. + method2(param1, param2): Description of method2. """ + + def __init__(self, attribute1, attribute2, attribute3): + """ + Initializes the ExampleClass with the given attributes. + + :param type attribute1: Description of attribute1. + :param type attribute2: Description of attribute2. + :param type attribute3: Description of attribute3. + """ + self.attribute1 = attribute1 + self.attribute2 = attribute2 + self.attribute3 = attribute3 ``` ## Comments