Enable starting driver from Fortran restart files for TC case #339

ajdas1 · 2022-09-20T21:48:33Z

Purpose

Allows a start from fortran restart files as needed to run the tropical cyclone initialization case.

Code changes:

Added GeneratedGridConfig and SerialboxGridConfig to pace.driver
Added grid_config attribute to DriverConfig, used to select grid initialization approach instead of relying on the state initialization to do so
Added FortranRestartConfig for state initialization
Can now use fortran restart files to initialize the model (tc case)
Possibility for grid transformation that allows for local refinement.
removed unused "ks" variable from fv3core and dataclass containers
added grid variables ee1, ee2, es1, and ew2 to GridData as they are needed by fv3core for baroclinic initialization
deleted difficult-to-maintain mock-based driver tests, they are difficult to maintain and don't add much coverage past the driver tests which actually run the code
removed unused stretch_grid attribute from Namelist class since Namelist is never used when defining grid initialization configuration
fv3core grid dataclasses now use Quantity instead of gt4py storages

Checklist

Before submitting this PR, please make sure:

You have followed the coding standards guidelines established at Code Review Checklist.
Docstrings and type hints are added to new and updated routines, as appropriate
All relevant documentation has been updated or added (e.g. README, CONTRIBUTING docs)
For each public change and fix in pace-util, HISTORY has been updated
Unit tests are added or updated for non-stencil code changes

Additionally, if this PR contains code authored by new contributors:

The names of all the new contributors have been added to CONTRIBUTORS.md

…tran

…_restart

elynnwu · 2022-10-10T18:11:05Z

.gitignore

+*.png
+*.nc


Do we actually want to ignore all png and nc files?

Technically no, I think we do have one of each committed into the repo that should be there. But adding to the gitignore doesn't prevent from adding files manually, and our scripts produce a fair bit of these that we don't want to commit. I could go either way but I'd lean towards having this in the gitignore, what do you think?

I'm okay with leaving it like this, just wanted to be careful when we do actually want to commit a png/nc file.

elynnwu · 2022-10-10T18:11:53Z

driver/pace/driver/diagnostics.py

+            "lat": grid_data.lat,
+            "lon": grid_data.lon,


elynnwu · 2022-10-10T18:18:04Z

driver/pace/driver/driver.py

+@dataclasses.dataclass()
+class RestartConfig:


What's the reason behind this being inside driver.py?

It should probably go in another file, I only put it here because all its code/logic was previously contained in this file. I'll see about splitting it out.

When I went to do this I remembered the reason is this is tightly coupled with DriverConfig, due to the fact that it writes an updated DriverConfig when writing the restart. Must be located in the same file.

Fair enough.

elynnwu · 2022-10-10T18:20:40Z

driver/pace/driver/driver.py

+    def cleanup(self):
+        logger.info("cleaning up driver")
+        self.config.restart_config.write_final_if_enabled(
            state=self.state,
-            comm=self.comm,
-            restart_path=restart_path,
-        )
-        self.restart.write_restart_config(
            comm=self.comm,
            time=self.time,
            driver_config=self.config,
-            restart_path=restart_path,
+            restart_path="RESTART",
        )


It looks to me that some of these don't need to be part of dace call back, not sure if this changes the behavior of dace.

There's a dace inhibitor only because if we do call this from dace it should be escaped in a callback, I don't think this is actually called from dace currently. Added it for safety. I can remove it if you would still prefer.

elynnwu · 2022-10-10T18:21:51Z

driver/pace/driver/grid.py

+@dataclasses.dataclass
+class GeneratedGridConfig(GridInitializer):
+    """
+    Configuration for baroclinic initialization.


Suggested change

Configuration for baroclinic initialization.

Grid calculated using metric terms

Adding something slightly different.

elynnwu · 2022-10-10T18:53:09Z

driver/pace/driver/state.py

+    is_fortran_restart = False
+    restart_files = fs.ls(path)
+    is_fortran_restart = any(


Do we need to default to False first?

No. Updated.

elynnwu · 2022-10-10T18:55:09Z

stencils/pace/stencils/testing/grid.py

+            # lon=self.bgrid1,
+            # lat=self.bgrid2,
+            # lon_agrid=self.agrid1,
+            # lat_agrid=self.agrid2,
+            # area=self.area,
+            # area_64=self.area_64,
+            # rarea=self.rarea,
+            # rarea_c=self.rarea_c,
+            # dx=self.dx,
+            # dy=self.dy,
+            # dxc=self.dxc,
+            # dyc=self.dyc,
+            # dxa=self.dxa,
+            # dya=self.dya,
+            # rdx=self.rdx,
+            # rdy=self.rdy,
+            # rdxc=self.rdxc,
+            # rdyc=self.rdyc,
+            # rdxa=self.rdxa,
+            # rdya=self.rdya,
+            # ee1=self.ee1,
+            # ee2=self.ee2,
+            # es1=self.es1,
+            # ew2=self.ew2,
+            # a11=self.a11,
+            # a12=self.a12,
+            # a21=self.a21,
+            # a22=self.a22,
+            # edge_w=self.edge_w,
+            # edge_e=self.edge_e,
+            # edge_s=self.edge_s,
+            # edge_n=self.edge_n,


Delete this

elynnwu · 2022-10-10T18:55:19Z

stencils/pace/stencils/testing/grid.py

+            # self.cosa,
+            # self.cosa_u,
+            # self.cosa_v,
+            # self.cosa_s,
+            # self.sina_u,
+            # self.sina_v,
+            # self.rsina,
+            # self.rsin_u,
+            # self.rsin_v,
+            # self.rsin2,


delete this

elynnwu · 2022-10-10T18:55:28Z

stencils/pace/stencils/testing/grid.py

+            # self.sin_sg1,
+            # self.sin_sg2,
+            # self.sin_sg3,
+            # self.sin_sg4,
+            # self.cos_sg1,
+            # self.cos_sg2,
+            # self.cos_sg3,
+            # self.cos_sg4,


delete this

elynnwu · 2022-10-10T18:58:32Z

tests/main/driver/test_restart_fortran.py

+    )
+
+    quantity_factory = QuantityFactory.from_backend(sizer=sizer, backend="numpy")
+    restart_dir = os.path.join(DIR, "../../../util/tests/data/c12_restart")


Would it be better to define pace dir at the top and use that throughout this file?

Co-authored-by: Elynn Wu <[email protected]>

…feature/fortran_restart

oelbert and others added 30 commits June 21, 2022 14:22

initial commit

056d0ec

first attempt at functions to restart from Fortran

efc69b5

added code in _restart_driver_state to call _overwrite_state_from_for…

c0e933a

…tran

fixed state traversal

c819291

Merge branch 'main' into feature/fortran_restart

c92702d

restartconfig now runs, but model crashes after

a01292c

lots of print statements

6c56f28

removed print statements - mostly

4e53988

modifying properties

d40d8bc

small changes

10677b1

updated properties

8970f5d

updated RESTART_PROPERTIES

72c326d

updated gt4py, buildenv

00ba2c8

Merge branch 'main' into feature/fortran_restart

0dfeec5

renamed pace physics

6b2ada9

pre-commit

5f1a1c2

writing test for restart

309ed12

check only dycore, tests

19f72f3

modified properties

2d0f4e6

begin generating grid from metric terms

14c3949

git pull on main

1b83fcf

Merge branch 'main' of github.com:ai2cm/pace into main

4ba9d75

add from_lat_lon method to MetricTerms

de74a75

metric_terms look good

5b04203

testing metric terms

ab93000

gitignore

46f9121

Merge branch 'feature/metric_terms_from_lat_lon' into feature/fortran…

9dcaa88

…_restart

next step

14b88d2

Merge branch 'main' of github.com:ai2cm/pace into main

5e041ce

added grid transformation - Schmidt

02f4df6

mcgibbon added 10 commits October 7, 2022 11:52

add workaround for grid still using storage in translate tests

8b1ecba

start fixing tests

cd0b401

Merge branch 'main' into fix/grid_origins

c0e59e8

Merge branch 'fix/grid_origins' into feature/fortran_restart

f7fb0da

fix restart mpi test

892f358

fix driver main test

4394baa

fix grid init 54rank test

52cece8

add cpu orchestration main test

c258dd5

attempt to fix dace_gpu failure on jenkins

4d9758c

Merge branch 'main' into feature/fortran_restart

63d973e

elynnwu reviewed Oct 10, 2022

View reviewed changes

mcgibbon and others added 10 commits October 17, 2022 18:25

update test_main circleci plan to use gt4py caching

b7ddb85

write version of split_quantity for storages

7c34196

fix config validation

f3e06eb

Merge branch 'main' into feature/fortran_restart

0862dd1

better fix for memoryview type error

8bee945

add gcloud_encoded_key context to test_main

58cf5dd

Update driver/pace/driver/initialization.py

38f5269

Co-authored-by: Elynn Wu <[email protected]>

response to review comments

4b72f99

use smaller data for test_main

42e6b66

Merge branch 'feature/fortran_restart' of github.com:ai2cm/pace into …

7bede1c

…feature/fortran_restart

elynnwu approved these changes Oct 18, 2022

View reviewed changes

mcgibbon added 2 commits October 18, 2022 22:45

Merge branch 'main' into feature/fortran_restart

5a17b70

remove system-dependent non-working test

9ec891e

mcgibbon changed the title ~~Feature/fortran restart~~ Enable starting driver from Fortran restart files for TC case Oct 18, 2022

update pace-util HISTORY

07b0240

mcgibbon approved these changes Oct 18, 2022

View reviewed changes

mcgibbon enabled auto-merge (squash) October 18, 2022 22:54

mcgibbon merged commit 243ff24 into main Oct 19, 2022

mcgibbon deleted the feature/fortran_restart branch October 19, 2022 00:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable starting driver from Fortran restart files for TC case #339

Enable starting driver from Fortran restart files for TC case #339

ajdas1 commented Sep 20, 2022 •

edited by mcgibbon

Loading

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 18, 2022

elynnwu Oct 10, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 18, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

elynnwu Oct 10, 2022

elynnwu Oct 10, 2022

elynnwu Oct 10, 2022

mcgibbon Oct 17, 2022

	Configuration for baroclinic initialization.
	Grid calculated using metric terms

		*.png
		*.nc

		@dataclasses.dataclass()
		class RestartConfig:

Enable starting driver from Fortran restart files for TC case #339

Enable starting driver from Fortran restart files for TC case #339

Conversation

ajdas1 commented Sep 20, 2022 • edited by mcgibbon Loading

Purpose

Code changes:

Checklist

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajdas1 commented Sep 20, 2022 •

edited by mcgibbon

Loading