Refactor fv3 runtime modules and image construction #185

nbren12 · 2020-03-17T22:35:21Z

As part of the effort to adjust the runfile for the one_step jobs, I realized that I could share the following functionality:

the docker image built by the prognostic run workflow
the online_modules directory used there.

After talking to @frodre and @oliverwm1, I decided to pull these routines into the top-level fv3net workspace. This involved the following changes:

build/version docker images in top level
- Start versioning fv3net for the purposes of building docker images
- Move docker image construction for a prognostic_run and fv3net images to the /docker folder.
- Delete the old contents of /docker which seem obsolete (last touched Oct 2019).
- add rules make push_images and make build_images
- Update the README.md
- updated yamls in workflow directories
move workflows/prognostic_c48_run/online_modules into fv3net/runtime. This module will now be available to other workflows.

Todo:

rerun diagnostics workflow in argo
rerun full end to end pipeline

Builds a docker image with fv3net, vcm, and fv3gfs-python installed.

nbren12 · 2020-03-17T22:49:37Z

Currently getting this error:

$ argo logs prognostic-run-diags-mtrbs-3453690810                                                                                                                                                               (fv3net)
Traceback (most recent call last):
  File "/opt/conda/envs/fv3net/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/opt/conda/envs/fv3net/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/jovyan/fv3net/fv3net/pipelines/save_prognostic_run_diags.py", line 75, in <module>
    resampled = ds.resample(time="3H", label="right").nearest()
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/resample.py", line 125, in nearest
    return self._upsample("nearest", tolerance=tolerance)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/resample.py", line 60, in _upsample
    return self._obj.reindex(method=method, *args, **kwargs)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/dataset.py", line 2483, in reindex
    **indexers_kwargs,
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/dataset.py", line 2514, in _reindex
    sparse=sparse,
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/alignment.py", line 546, in reindex_variables
    int_indexer = get_indexer_nd(index, target, method, tolerance)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/core/indexing.py", line 103, in get_indexer_nd
    flat_indexer = index.get_indexer(flat_labels, method=method, tolerance=tolerance)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/pandas/core/indexes/base.py", line 2740, in get_indexer
    indexer = self._get_nearest_indexer(target, limit, tolerance)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/pandas/core/indexes/base.py", line 2820, in _get_nearest_indexer
    left_distances = np.abs(self[left_indexer] - target)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/coding/cftimeindex.py", line 444, in __sub__
    return CFTimeIndex(np.array(self) - other)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/coding/cftimeindex.py", line 248, in __new__
    assert_all_valid_date_type(data)
  File "/opt/conda/envs/fv3net/lib/python3.7/site-packages/xarray/coding/cftimeindex.py", line 208, in assert_all_valid_date_type
    "objects. Got object of {}.".format(date_type)
TypeError: CFTimeIndex requires cftime.datetime objects. Got object of <class 'pandas._libs.tslibs.timedeltas.Timedelta'>.

nbren12 · 2020-03-17T22:56:16Z

@spencerkclark ^^^^ seems up your alley. Thoughts?

spencerkclark · 2020-03-17T23:06:53Z

Yup was just going to suggest pinning pandas; looks like you figured it out. This should be straightened out in the next release of xarray.

nbren12 · 2020-03-17T23:14:38Z

haha. I saw the only difference between the images was pandas 1.0.2 vs 1.01. Why a "patch" update would break a release I have no idea.

nbren12 · 2020-03-17T23:16:27Z

I think I've been in dependency hell a lot lately.

spencerkclark · 2020-03-17T23:25:22Z

Yeah they made some changes that broke indexing with the "nearest" method with a CFTimeIndex: pydata/xarray#3751. It's somewhat understandable since they don't have any tests for how we're subclassing pandas.Index. We made changes in xarray to address this, but we haven't issued a release yet.

Pandas may well address this on their end too...we'll see. Kind of a pain though that they went forward with their release anyway (we brought this to their attention ahead of time).

nbren12 · 2020-03-17T23:41:30Z

Yah. I think this is unavoidable when using open source tools, honestly I put the blame on conda for not having a great version "locking" solution like poetry or pipfile. With that it would much easier to pin our versions without over-specifying the environment.yml.

nbren12 · 2020-03-17T23:44:38Z

It's just annoying that I've burned by upstream dependencies of our dependencies twice in two days.

nbren12 · 2020-03-18T00:27:06Z

Compute the R2 of the baseline physics.

frodre

Nice and clean rework. Thanks, @nbren12. One requested change for the fv3net Makefile, but otherwise LGTM.

frodre · 2020-03-18T18:20:42Z

Makefile

+	pip wheel --no-deps external/vcm
+
+# pattern rule for building docker images
+build_image_%:


Makefile

oliverwm1

Looks good. I have one comment/suggestion about a function name. More generally, if we're moving to have some general purpose functions that can be used in runfiles, seems like we might want to formalize the "runfile config" idea. Possibly we could enforce that any configuration options that are read in by runfiles are under a runfile item in fv3config.yml.

oliverwm1 · 2020-03-18T21:25:43Z

fv3net/runtime/config.py

@@ -10,7 +10,7 @@ class dotdict(dict):
    __delattr__ = dict.__delitem__


-def get_config():
+def get_runfile_config():


What do you mean by a runfile config? Should we formalize this somehow? I see the value of the concept, but given what this function does it seems like get_sklearn_config or something would be a better name.

I agree with you, and have removed this function in #193. I think i will merge this as is though, just to get the ball rolling.

…ine-modules

Create CONTRIBUTING.md

nbren12 added 9 commits March 17, 2020 19:18

Refactore online_modules to fv3net

8ca8819

black

5909c73

Create us.gcr.io/vcm-ml/prognostic_run:v0.1.0

cee28fd

Builds a docker image with fv3net, vcm, and fv3gfs-python installed.

Refactor us.gcr.io/vcm-ml/fv3net image build code

305629f

Add build_images makefile target

ff9f938

Add __version__ to fv3net init

b31a099

update prognostic_run_diags configuration

8f9cb3c

black

31930d9

update readme

f994766

nbren12 changed the title ~~Refactore fv3 runtime modules and image construction~~ Refactor fv3 runtime modules and image construction Mar 17, 2020

nbren12 added 2 commits March 17, 2020 22:42

Fix table in README

871be05

fix yaml bug

db16a39

pin pandas version to 1.0.1

47d69e3

frodre approved these changes Mar 18, 2020

View reviewed changes

oliverwm1 approved these changes Mar 18, 2020

View reviewed changes

nbren12 added 2 commits March 21, 2020 00:08

Merge remote-tracking branch 'origin/master' into refactor/fv3net-onl…

3c91902

…ine-modules

correct versions in argo

87a9127

nbren12 merged commit 598cb26 into master Mar 21, 2020

nbren12 deleted the refactor/fv3net-online-modules branch March 21, 2020 00:24

nbren12 mentioned this pull request Mar 24, 2020

Feature/circleci fv3net build push #192

Merged

spencerkclark pushed a commit that referenced this pull request May 7, 2021

Merge pull request #185 from jacobtomlinson/patch-1

3453354

Create CONTRIBUTING.md

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor fv3 runtime modules and image construction #185

Refactor fv3 runtime modules and image construction #185

nbren12 commented Mar 17, 2020 •

edited

Loading

nbren12 commented Mar 17, 2020 •

edited

Loading

nbren12 commented Mar 17, 2020

spencerkclark commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 17, 2020

spencerkclark commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 18, 2020

frodre left a comment

frodre Mar 18, 2020

oliverwm1 left a comment

oliverwm1 Mar 18, 2020

nbren12 Mar 21, 2020

Refactor fv3 runtime modules and image construction #185

Refactor fv3 runtime modules and image construction #185

Conversation

nbren12 commented Mar 17, 2020 • edited Loading

nbren12 commented Mar 17, 2020 • edited Loading

nbren12 commented Mar 17, 2020

spencerkclark commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 17, 2020

spencerkclark commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 17, 2020

nbren12 commented Mar 18, 2020

frodre left a comment

Choose a reason for hiding this comment

frodre Mar 18, 2020

Choose a reason for hiding this comment

oliverwm1 left a comment

Choose a reason for hiding this comment

oliverwm1 Mar 18, 2020

Choose a reason for hiding this comment

nbren12 Mar 21, 2020

Choose a reason for hiding this comment

nbren12 commented Mar 17, 2020 •

edited

Loading

nbren12 commented Mar 17, 2020 •

edited

Loading