Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
  • Loading branch information
peverwhee committed Dec 9, 2024
2 parents 6d61829 + c24b294 commit 7acca22
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 5 deletions.
2 changes: 1 addition & 1 deletion docs/atmospheric_physics/Tagging-Instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This page lists instructions for how to create a new atmospheric_physics tag. A

1. Tags should always be annotated (no lightweight tags).
2. Tags should follow the naming convention listed [below](#tag-naming-conventions).
3. Tags should point to merge commits into the main branch.
3. All merge commits into the main branch should be tagged.
4. Tags can only be created by people who have write access.

## How to create a git tag via the command line
Expand Down
9 changes: 6 additions & 3 deletions docs/atmospheric_physics/development_workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ The general workflow for adding a feature, bug-fix, or modification to atmospher

If you need an official tag for your new additions, then once your `development` PR has been merged you will need to do the following:

1. **Open a PR that merges the atmospheric_physics `development` branch into `main`. Ensure that the PR description lists every PR that went into `development` since the last update to `main`.**
1. **Open a PR that merges the atmospheric_physics `development` branch into `main`. Ensure that the PR description lists the title and number of every PR that went into `development` since the last update to `main`.**
2. **Fix any failing tests. This includes tests on the target host models that will be using the new tag.**
3. **Merge (do not squash!) the PR.**
4. **Tag the new merge commit.**
4. **[Tag](Tagging-Instructions.md) the new merge commit.**

## Workflow details

Expand Down Expand Up @@ -224,7 +224,10 @@ At a minimum, unit tests for ESCOMP source code should:

**11. Update the `NamesNotInDictionary.txt` file using the instructions [below](#updating-namesnotindictionarytxt-file).**

**12. Once all reviewers sign off on your modifications, then the PR will be squashed and merged. Congratulations! Your code is now in atmospheric_physics!**
**12. Once all reviewers sign off on your modifications, then the PR can be merged. Congratulations! Your code is now in atmospheric_physics!**

- If the PR is to `develop`, you should select "Squash and Merge"
- If the PR is to `main`, DO NOT squash and instead select "Merge Commit"

### Updating NamesNotInDictionary.txt file

Expand Down
157 changes: 156 additions & 1 deletion docs/conversion/walkthrough.md
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,162 @@ Bug gets extremely lucky (or maybe he's just really skilled) and there are no di
Bug commits his changes to his fork/branch of the three repos: CAM, CAM-SIMA, atmospheric_physics.

### Unit testing
Bug adds PfUnit tests for his newly-created scheme per the [atmospheric_physics development workflow](../atmospheric_physics/development_workflow.md#5-unit-testing).

Lets say you added a utility function in `physics_tendency_updaters.F90
` to apply the pressure tendency of the atmosphere:

```fortran
subroutine apply_tendency_of_air_pressure_run(nz, p_tend, pressure, dPdt_total, &
dt, errcode, errmsg)
! Dummy arguments
integer, intent(in) :: nz ! Num vertical layers
real(kind_phys), intent(in) :: p_tend(:,:) ! pressure tendency
real(kind_phys), intent(inout) :: pressure(:,:) ! air pressure
real(kind_phys), intent(inout) :: dPdt_total(:,:) ! total temp. tendency
real(kind_phys), intent(in) :: dt ! physics time step
integer, intent(out) :: errcode
character(len=512), intent(out) :: errmsg
! Local variable
integer :: klev
errcode = 0
errmsg = ''
do klev = 1, nz
pressure(:, klev) = pressure(:, klev) + (p_tend(:, klev) * dt)
dPdt_total(:, klev) = dPdt_total(:, klev) + p_tend(:, klev)
end do
end subroutine apply_tendency_of_air_pressure_run
```

Then you would need to check that the file you've modified is being built by the tests. You can check this in `test/unit-test/CMakeLists.txt` and see that we have:

```cmake
set(UTILITIES_SRC
../../schemes/utilities/state_converters.F90
../../schemes/utilities/static_energy.F90
../../schemes/utilities/physics_tendency_updaters.F90
include/ccpp_kinds.F90
)
```

Since the file is being built, we don't have to add to this list.

Next we look for `test/unit-test/tests/utilities/CMakeLists.txt` to see if there is a matching test file:

```bash
$ ls test/unit-test/tests/utilities/
CMakeLists.txt
test_state_converters.pf
$ cat test/unit-test/tests/utilities/CMakeLists.txt
add_pfunit_ctest(utilities_tests
TEST_SOURCES test_state_converters.pf
LINK_LIBRARIES utilities
)
```

Because there is no matching file or test, we need to create it:

```bash
$ cat > test/unit-test/tests/utilities/test_physics_tendency_updaters.pf << EOF
@test
subroutine test_pressure_tendency_update()
use funit
use physics_tendency_updaters
end subroutine test_pressure_tendency_update
EOF
```

And add the ability to build/run it from the test harness by updating `test/unit-test/tests/utilities/CMakeLists.txt`:

```cmake
add_pfunit_ctest(utilities_tests
TEST_SOURCES test_state_converters.pf test_physics_tendency_updaters.pf
LINK_LIBRARIES utilities
```
where `test_physics_tendency_updaters.pf` has been added to the `TEST_SOURCES` list.

Now, to actually test the function, we can try something like:

```fortran
@test
subroutine test_pressure_tendency_update()
use funit
use ccpp_kinds, only: kind_phys
use physics_tendency_updaters
# Declare and setup test data with known results
integer, parameter :: ncol = 3 ! Num columns
integer, parameter :: nz = 3 ! Num vertical layers
integer, parameter :: errcode = 0
character(len=512), parameter :: errmsg = ''
real(kind_phys) :: p_tend(ncol, nz) ! pressure tendency
real(kind_phys) :: pressure(ncol, nz) ! air pressure
real(kind_phys) :: dPdt_total(nol, nz) ! total temp. tendency
real(kind_phys) :: dt ! physics time step
pressure = 1
p_tend = 1
dPdt_total = 0
dt = 1
call apply_tendency_of_air_pressure_run(nz, p_tend, pressure, &
dPdt_total, dt, &
errcode, errmsg)
@assertEqual(2, pressure)
@assertEqual(1, dPdt_total)
@assertEqual(0, errcode)
@assertEqual('', errmsg)
end subroutine test_pressure_tendency_update
```

The values should be scientifically relevant to test a valid physics case or be set up to test edge cases that the subroutine must support.

Notice that we are only testing `intent(out)` and `intent(inout)` as these are the only values that can change.

Assuming you have built the test framework according to the [instructions](../atmospheric_physics/development_workflow.md#5-unit-testing), you can now run:

```bash
$ cmake \
-DCMAKE_PREFIX_PATH=<PATH_TO_PFUNIT>/build/installed \
-DATMOSPHERIC_PHYSICS_ENABLE_CODE_COVERAGE=ON \
-B./build \
-S./test/unit-test
$ cd build
$ make
$ ctest -V --output-on-failure
```

After which you should see output similar to:

```
1: Start: <test_state_converters_suite.test_temp_to_potential_temp>
1: . end: <test_state_converters_suite.test_temp_to_potential_temp>
1:
1: Time: 0.000 seconds
1:
1: OK
1: (1 test)
1:
2: Start: <test_physics_tendency_updaters_suite.test_pressure_tendency_update>
2: . end: <test_physics_tendency_updaters_suite.test_pressure_tendency_update>
2:
2: Time: 0.000 seconds
2:
2: OK
2: (1 test)
2/2 Test #1: utilities_tests .................. Passed 0.00 sec
100% tests passed, 0 tests failed out of 1
```

If 100% of the tests pass, you may proceed to the next section.

### Pull Requests
- He opens a PR into atmospheric_physics (target: `development` branch), goes through the review process, updates the NamesNotInDictionary.txt file, and then commits the PR when approvals are received. He then opens a PR from `development` to `main` and makes a tag (incrementing the minor version) when that is merged.
Expand Down

0 comments on commit 7acca22

Please sign in to comment.