-
Notifications
You must be signed in to change notification settings - Fork 24
Turn main unit test & lint on PR, logger clean up [NASA:Update] #15
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
Changes from all commits
6d78659
0a3e857
0a8d705
3b73d71
a68d160
33ba53f
8968698
2327cbe
cb4ec5f
7348922
e234d16
131a2af
dce3fb7
8982542
da2f902
f2799d8
27fae1c
2f8ebac
5d9e0a0
b8edbf2
f54b231
81d00ce
4891d56
fafbfc7
4fc5b4d
2245027
4f8fdc3
47421a0
d51bc11
6bdd595
80cbb01
40f2440
cae25a9
915993e
c3e355c
08f6e68
cde11e8
9e6bbb6
6695cec
7e449cd
d5d0ee5
6f8bed5
c4d5f41
adbddf8
e8c669c
d0e8be6
69429b4
1a7c642
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: "Lint" | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] | ||
|
|
||
| jobs: | ||
| lint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout Pace repository | ||
| uses: actions/checkout@v3.5.2 | ||
| with: | ||
| submodules: 'recursive' | ||
| - name: Step Python 3.8.12 | ||
| uses: actions/setup-python@v4.6.0 | ||
| with: | ||
| python-version: '3.8.12' | ||
| - name: Install OpenMPI for gt4py | ||
| run: | | ||
| sudo apt-get install libopenmpi-dev | ||
| - name: Install Python packages | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install -r requirements_dev.txt -r requirements_lint.txt | ||
| - name: Run lint via pre-commit | ||
| run: | | ||
| pre-commit run --all-files |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: "Main unit tests" | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] | ||
|
|
||
| jobs: | ||
| main_unit_tests: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout Pace repository | ||
| uses: actions/checkout@v3.5.2 | ||
| with: | ||
| submodules: 'recursive' | ||
| - name: Step Python 3.8.12 | ||
| uses: actions/setup-python@v4.6.0 | ||
| with: | ||
| python-version: '3.8.12' | ||
| - name: Install OpenMPI for gt4py | ||
| run: | | ||
| sudo apt-get install libopenmpi-dev | ||
| - name: Install Python packages | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install -r requirements_dev.txt | ||
| - name: Run all main tests | ||
| run: | | ||
| pytest -x tests/main | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ | |
|
|
||
|
|
||
| def test_geos_wrapper(): | ||
|
|
||
| namelist_dict = { | ||
| "stencil_config": { | ||
| "compilation_config": { | ||
|
|
@@ -82,7 +81,12 @@ def test_geos_wrapper(): | |
| comm = NullComm(rank=0, total_ranks=6, fill_value=0.0) | ||
| backend = "numpy" | ||
|
|
||
| wrapper = fv3core.GeosDycoreWrapper(namelist, comm, backend) | ||
| wrapper = fv3core.GeosDycoreWrapper( | ||
| namelist=namelist, | ||
| comm=comm, | ||
| backend=backend, | ||
| bdt=namelist_dict["dt_atmos"], | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Realizing now that we're passing this in twice, once via the namelist and once directly here. Probably outside the scope of this PR to reduce that duplication, but in principle we could add a bit to the wrapper's
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. The wrapper could actually be broken a bit more. I am thinking there's "re-usable" parts for any model integration then things that are GEOS. We need more thinking and clean up here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once I confirm things with one of the NOAA-GFDL github domain managers tomorrow that we are good on the github action front, I'll merge this in. |
||
| ) | ||
| nhalo = 3 | ||
| shape_centered = ( | ||
| namelist["nx_tile"] + 2 * nhalo, | ||
|
|
@@ -191,31 +195,33 @@ def test_geos_wrapper(): | |
| ) | ||
| diss_estd = np.ones(shape_centered) | ||
|
|
||
| output_dict = wrapper( | ||
| u, | ||
| v, | ||
| w, | ||
| delz, | ||
| pt, | ||
| delp, | ||
| q, | ||
| ps, | ||
| pe, | ||
| pk, | ||
| peln, | ||
| pkz, | ||
| phis, | ||
| q_con, | ||
| omga, | ||
| ua, | ||
| va, | ||
| uc, | ||
| vc, | ||
| mfxd, | ||
| mfyd, | ||
| cxd, | ||
| cyd, | ||
| diss_estd, | ||
| timings = {} | ||
| output_dict, timings = wrapper( | ||
| timings=timings, | ||
| u=u, | ||
| v=v, | ||
| w=w, | ||
| delz=delz, | ||
| pt=pt, | ||
| delp=delp, | ||
| q=q, | ||
| ps=ps, | ||
| pe=pe, | ||
| pk=pk, | ||
| peln=peln, | ||
| pkz=pkz, | ||
| phis=phis, | ||
| q_con=q_con, | ||
| omga=omga, | ||
| ua=ua, | ||
| va=va, | ||
| uc=uc, | ||
| vc=vc, | ||
| mfxd=mfxd, | ||
| mfyd=mfyd, | ||
| cxd=cxd, | ||
| cyd=cyd, | ||
| diss_estd=diss_estd, | ||
| ) | ||
|
|
||
| assert isinstance(output_dict["u"], np.ndarray) | ||
Uh oh!
There was an error while loading. Please reload this page.