Refactor `omPriorVerify.py` into tests #14

crdanielbusch · 2024-05-21T13:38:11Z

Description

Remove omPriorVerify.py call from omPrior.py
Refactor tests from omPriorVerify.py in test suite - see tests 005 and 006
Combine all tests in one module
Add a fixtures that downloads input files
Add a fixture that runs the whole process and references the input files fixture
Fix CI
Add a time out for the tests (currently at 10 min)

Checklist

Please confirm that this pull request has done the following:

Tests added
Documentation added (where applicable)

Notes

This reverts commit 819f890.

…rify-emissions-test # Conflicts: # tests/test_download_data.py # tests/test_om_prior.py

crdanielbusch · 2024-05-29T09:08:19Z

tests/test_om_prior.py

+# TODO Update this test when file structure is clear.
+# This test ensures that the grid size for all input files is 10 km.
+def test_006_grid_size_for_geo_files(root_dir, monkeypatch) :
+    expected_cell_size = 10000


I am not sure what exactly to test here. Right now the test ensure that the grid size for geo_em.d01.nc, GRIDCRO2D_1, GRIDDOT2D_1 is 10 km.

My initial thought was lat/lons, but then I realised that the GRIDCRO/GRIDDOT are a subset of the WRF domain.

That could be a test for after omCreateDomainInfo.py is run, but not applicable here

lewisjared

Looks good. There are a few suggestions for other things to test. Whether this comes in the PR or later is up to you.

lewisjared · 2024-05-29T09:43:28Z

tests/test_om_prior.py


-def test_full_process(num_regression, root_dir, monkeypatch):
+    os.remove(filepath)


Given that filepath uses the tmpdir fixture, the files are automatically cleaned up so don't need to be removed. Therefore this line isn't needed

If you really wanted to be sure that something is cleaned up after the test you should put it in a try finally block. Otherwise the test might exit early before the remove call. Or you could have a fixture that uses yield like you did above

try: create_file() this_might_fail() finally: # This is always called even if an exception is raised in `this_migh_fail()` if filepath.exists(): filepath.remove()

lewisjared · 2024-05-29T10:01:40Z

tests/test_om_prior.py

+
+
+# Fixture to download and later remove all input files
+@pytest.fixture(scope="session")


This fixture can probably move into conftest.py as it could be used in other places. Also since it is session scoped even if you use it in multiple test modules it will still only run once

lewisjared · 2024-05-29T10:19:29Z

tests/test_om_prior.py

+# TODO Update this test when file structure is clear.
+# This test ensures that the grid size for all input files is 10 km.
+def test_006_grid_size_for_geo_files(root_dir, monkeypatch) :
+    expected_cell_size = 10000


My initial thought was lat/lons, but then I realised that the GRIDCRO/GRIDDOT are a subset of the WRF domain.

That could be a test for after omCreateDomainInfo.py is run, but not applicable here

scripts/omPrior.py

lewisjared · 2024-05-29T10:21:17Z

tests/test_om_prior.py

+
+@pytest.fixture(scope="session")
+def output_domain_file(root_dir,
+                       input_files


This doesn't look ruffed

lewisjared · 2024-05-29T10:51:12Z

tests/test_om_prior.py

+    filepath_in_domain = os.path.join(root_dir, "inputs/om-domain-info.nc")
+
+    # Generated by scripts/omPrior.py
+    filepath_out_domain = os.path.join(root_dir, "outputs/out-om-domain-info.nc")


Can we add a new test that verifies that this output file has the same lat/lons that are in the CRO/DOT files. filepath_in_domain should have the same dimensions as well.

Is it possible that GRIDCRO2D_1 and GRIDDOT2D_1 have different dimensions?

In this test I compare COL and ROW of GRIDCRO2D_1 and GRIDDOT2D_1 with inputs/om-domain-info.nc.

The number of columns and rows in GRIDCRO2D_1 is one below the number of columns and rows in GRIDDOT2D_1:

GRIDCRO2D_1 contains the attributes ROW: 430, COL: 454

and

GRIDDOT2D_1 contains the attributes ROW: 431, COL: 455.

In om-domain-info.nc there are the attributes ROW: 430 and ROW_D: 431.
So I can compare ROW with GRIDCRO2D_1 and ROW_D with GRIDDOT2D_1?

The test that I am adding looks like this. It's passing, but I am not sure if the test makes sense.

def test_007_compare_in_domain_with_cro_dot_files(input_domain_xr, cro_xr, dot_xr): assert dot_xr.NCOLS == input_domain_xr.COL_D.size assert dot_xr.NROWS == input_domain_xr.ROW_D.size assert cro_xr.NCOLS == input_domain_xr.COL.size assert cro_xr.NROWS == input_domain_xr.ROW.size

WRF and CMAQ use a staggered grid (https://amps-backup.ucar.edu/information/configuration/wrf_grid_structure.html) where depending on the quantity that you are measuring the grid points are slightly different. Things like temperature/pressure are defined at the center of a grid cell, where as other quantities such as wind are defined at the edges of a grid cell. If you count the number of edges in a grid along a x or y it is one larger than the number of cells in x or y.

The CRO/DOT files are generated by MCIP (?) from the WRF domain and define the domain used by CMAQ.
The CRO files define the latitude centers where as the DOT files define the corners hence why ROW_D = ROW + 1. I don't know why they are named as they are 🤷

The CMAQ domain is a little smaller than the WRF domain because the 5 grid cells around the edge of the domain are excluded. I can talk you through why that is another day.

According to this website, CRO is short for cross and dot.

An image of the CMAQ grid: https://www.cmascenter.org/ioapi/documentation/all_versions/html/GRIDS.jpg
and the interaction between the grid and "thickened boundary" https://www.cmascenter.org/ioapi/documentation/all_versions/html/THKBDY.jpg. The outside is the WRF domain, but the inner cells labelled "main grid" is the CMAQ grid

Ok thanks. I think I have understood it to some degree.

Is the difference between CMAQ domain and WRF domain (the thickened boundary) unrelated to the mass grid / staggered grid problem? And it wouldn't be relevant to the tests here anyway, would it?

I've added the tests 007 and 008.

Correct. It is unrelated. If you compared the grid sizes in the raw input domain geo_em.d01.nc you would see that they don't align with the CMAQ grid

.github/actions/setup/action.yaml

prayner · 2024-05-30T02:03:48Z

I'm worried about hard coding a 10km test since subdomains will be finer resolution -- Peter Rayner (he/him) My work days are Monday-Thursday, responses at other times might be slow. The Superpower Institute <https://www.superpowerinstitute.com.au/> Level 3 105 Victoria St. Fitzroy 3065 Transparent, actionable and auditable data on Australia's methane emissions <https://openmethane.org> mobile +61 402 752 379 zoom id 4431343191, join at <https://unimelb.zoom.us/j/4431343191?pwd=a1E5Z3JEOTRVQUJsaVdRbVUvR1QyZz09> mail-to: ***@***.*** google scholar: <https://scholar.google.com.au/citations?user=H3up71wAAAAJ&hl=en> I acknowledge the Traditional Custodians of the land on which I work, the Wurundjeri people of the Kulin nation, and pay my respect to their Elders, past and present I am sending this email when convenient for me, please only respond when convenient for you

Co-authored-by: Jared Lewis <[email protected]>

lewisjared · 2024-05-30T08:40:57Z

I'm worried about hard coding a 10km test since subdomains will be
finer resolution

@prayner In this case we are testing that the example domain that is checked into the repo is consistent. These tests aren't run for every new domain (maybe they should eventually)

test livestock CH4 in bounds of total agriculture

bd25fe0

crdanielbusch changed the title ~~Refactor omPriorVerify.py into tests~~ Refactor omPriorVerify.py into tests May 21, 2024

crdanielbusch self-assigned this May 21, 2024

Daniel Busch added 19 commits May 22, 2024 16:21

refactor tests

c48236b

comments

4e700d3

fixture scope

f78affa

fixture scope

4cbdf90

test numbers

20cf6d6

test cdsapi connection

2f264e8

use local file for GFAS test

819f890

cdsapirc

ac0fc34

Revert "use local file for GFAS test"

66a7bfa

This reverts commit 819f890.

ci step

2070a50

tests: 007

f741541

tests all

3294486

tests pass locally

1d700d7

Merge remote-tracking branch 'refs/remotes/origin/main' into prior-ve…

78514e4

…rify-emissions-test # Conflicts: # tests/test_download_data.py # tests/test_om_prior.py

update fixture scopes

cfe299c

combine fixtures

d422a9b

fixture tear down

8b2b63f

fixtures

f57b5bc

clean up tests

0f01aa2

crdanielbusch requested a review from lewisjared May 29, 2024 09:05

crdanielbusch commented May 29, 2024

View reviewed changes

lewisjared approved these changes May 29, 2024

View reviewed changes

lewisjared reviewed May 29, 2024

View reviewed changes

.github/actions/setup/action.yaml Outdated Show resolved Hide resolved

aethr mentioned this pull request May 29, 2024

Update GitHub secrets for CDS API #15

Closed

Update .github/actions/setup/action.yaml

17e6e35

Co-authored-by: Jared Lewis <[email protected]>

Daniel Busch added 2 commits May 30, 2024 11:59

tests for CRO/DOT file dimensions

7d25a2d

clean up

a65fe5a

crdanielbusch merged commit e585743 into main May 30, 2024
1 check passed

crdanielbusch deleted the prior-verify-emissions-test branch May 30, 2024 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `omPriorVerify.py` into tests #14

Refactor `omPriorVerify.py` into tests #14

crdanielbusch commented May 21, 2024 •

edited

Loading

crdanielbusch May 29, 2024

lewisjared May 29, 2024

lewisjared left a comment

lewisjared May 29, 2024

lewisjared May 29, 2024

lewisjared May 29, 2024

lewisjared May 29, 2024

lewisjared May 29, 2024

crdanielbusch May 30, 2024

lewisjared May 30, 2024

lewisjared May 30, 2024

crdanielbusch May 30, 2024 •

edited

Loading

lewisjared May 30, 2024

prayner commented May 30, 2024 via email

lewisjared commented May 30, 2024


		def test_full_process(num_regression, root_dir, monkeypatch):
		os.remove(filepath)



		# Fixture to download and later remove all input files
		@pytest.fixture(scope="session")

Refactor omPriorVerify.py into tests #14

Refactor omPriorVerify.py into tests #14

Conversation

crdanielbusch commented May 21, 2024 • edited Loading

Description

Checklist

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewisjared left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crdanielbusch May 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prayner commented May 30, 2024 via email

lewisjared commented May 30, 2024

Refactor `omPriorVerify.py` into tests #14

Refactor `omPriorVerify.py` into tests #14

crdanielbusch commented May 21, 2024 •

edited

Loading

crdanielbusch May 30, 2024 •

edited

Loading