fix dataflow submission #1691

nbren12 · 2022-03-08T06:08:36Z

master is failing because the dependencies in workflows/dataflow/setup.py were incompatible with
constraints.txt. Dataflow now supports running via a docker image, which will be
easier to ensure a consistent environment.

switch to using docker for dataflow jobs
Switch to latest dataflow version. Our current one has been deprecated.
ensure dependencies are compatible with the versions installed in the dataflow
base image

These dependencies mostly match our stack, but did force some updates to
libraries in the google stack like tensorflow and the google-cloud-* packages.

Resolves #1678

The dependencies in `workflows/dataflow/setup.py` were incompatible with constraints.txt. Dataflow now supports running via a docker image, which will be easier to ensure a consistent environment. * switch to using docker for dataflow jobs * ensure dependencies are compatible with the versions installed in the dataflow base image These dependencies mostly match our stack, but did force some updates to libraries in the google stack like tensorflow and the google-cloud-* packages. Resolves #1678

* regression data needed updating (probably bc of new numpy version) * fix newly enforced behavior that np.pad cannot pad int data with np.nan

nbren12 · 2022-03-08T06:47:18Z

Looks like we need to

update prognostic run regression data
update make test_regression data. @brianhenn I actually cannot run these tests locally. I get this complaint:

E _pytest.config.ConftestImportFailure: (local('/home/noahb/workspace/ai2cm/fv3net/_build/pip_packages/lib/python3.8/site-packages/matplotlib/tests/conftest.py'), (<class 'OSError'>, OSError('The baseline image directory does not exist. This is most likely because the test data is not installed. You may need to install matplotlib from source to get the test data.'), <traceback object at 0x7fbddd98eb00>))

Do you have a workaround?

nbren12 · 2022-03-08T23:04:40Z

@spencerkclark The regression tests are failing because some of the coarsening routines want to pad an array of ints with
(np.nan), an operation that I don't think newer versions of numpy allows any longer. The int arrays are usually the coordinates.

I've been tracking down a lot of errors with this PR, so I would definitely would appreciate your help fixing that. Feel free to push directly here if you have any quick fixes.

This ensures that the dummy coordinates we construct during the vertical remapping process have dtype np.float32 from the start. Our coarsening procedure of these coordinates requires they be of a floating point type.

for some reason these tests fail when run via the `coverage` utility. We don't look at coverage stats much, so I took the expedient option and removed it for this small number of tests.

nbren12 · 2022-03-09T23:00:55Z

The test_unit failure looks like an unrelated network access issue.

nbren12

All new deps are ok.

nbren12 · 2022-03-08T06:09:58Z

constraints.txt

@@ -18,93 +18,110 @@ astor==0.8.1
 astunparse==1.6.3
 async-generator==1.10
 async-timeout==3.0.1
-attrs==19.3.0
-avro-python3==1.9.2.1
+atomicwrites==1.4.0


nbren12 · 2022-03-08T06:10:33Z

constraints.txt

 babel==2.8.0
 backcall==0.2.0
 backoff==1.10.0
 base58==2.1.0
+beautifulsoup4==4.10.0


nbren12 · 2022-03-08T06:11:20Z

constraints.txt

 bleach==3.3.0
 blinker==1.4
 bokeh==2.2.3
+bs4==0.0.1


not a real package. is related to beautifulsoup4 above. https://pypi.org/project/bs4/

nbren12 · 2022-03-08T06:11:40Z

constraints.txt

 cffi==1.14.4
 cfgv==3.2.0
 cftime==1.2.1
 chardet==3.0.4
+charset-normalizer==2.0.11


nbren12 · 2022-03-08T06:12:03Z

constraints.txt

 colorcet==2.0.6
 commonmark==0.9.1
 conda-lock==0.4.1
 configparser==5.0.2
 crcmod==1.7
 cryptography==36.0.1
 cycler==0.10.0
+cython==0.29.27


nbren12 · 2022-03-09T23:11:07Z

constraints.txt

 testpath==0.4.4
+tf-estimator-nightly==2.8.0.dev2021122109


nbren12 · 2022-03-09T23:11:20Z

constraints.txt

 testpath==0.4.4
+tf-estimator-nightly==2.8.0.dev2021122109
+threadpoolctl==3.1.0


nbren12 · 2022-03-09T23:11:47Z

constraints.txt

-typing-inspect==0.6.0
+typing-extensions==4.0.1
+typing-inspect==0.7.1
+typing-utils==0.1.0


nbren12 · 2022-03-09T23:12:06Z

constraints.txt

 tzlocal==2.1
-urllib3==1.25.10
+uritemplate==4.1.1


bsd-3 or apache 2

nbren12 · 2022-03-09T23:12:27Z

constraints.txt

 xarray==0.19.0
 xgcm==0.6.1
 xmltodict==0.12.0
 xpartition==0.2.0
 yarl==1.6.3
 yq==2.11.0
 zarr==2.7.0
+zipp==3.7.0


spencerkclark

Looks good to me. Nice that we can use a docker image for dataflow now.

spencerkclark · 2022-03-10T14:07:46Z

external/loaders/tests/test__base.py

@@ -76,13 +76,10 @@ def test_MultiDatasetMapper_keys(multi_dataset_mapper, expected_keys):


 def test_MultiDatasetMapper_value(multi_dataset_mapper, datasets):
-    single_time = datasets[0][TIME_NAME].isel({TIME_NAME: 0}).item()
-    time_key = pd.to_datetime(single_time).strftime(TIME_FMT)
+    single_time = pd.to_datetime(datasets[0][TIME_NAME].isel({TIME_NAME: 0}).item())


Oh yeah, the fact that calling item() on a np.datetime64 array returns an integer rather than a np.datetime64 scalar is weird, but a known issue (pydata/xarray#3256). This is definitely the right thing to do (I probably should have written things this way to begin with).

I think with this change we could probably still use sel down below, but the meaning of this test is still the same with isel, so there's no reason to change it. Interesting that it worked before, but now no longer works (I guess pandas used to automatically cast integer indexers to a DatetimeIndex to np.datetime64 values, and now it no longer does).

i think it was able to look things up with the int. Definitely outside of my expertise.

nbren12 added 5 commits March 7, 2022 22:07

add dockerfile

97b462c

fix to be squashed

515c400

update lint python version to 3.8

77c8e66

update vcm tests

4186e56

* regression data needed updating (probably bc of new numpy version) * fix newly enforced behavior that np.pad cannot pad int data with np.nan

nbren12 added 7 commits March 8, 2022 10:52

update fv3fit regression data

3f8a1e0

update prognostic run regression data

0906cc5

update holoviews

8062381

update fv3fit regression data

5666166

fix apache beam groupby

5557454

fix time indexing in loaders test

4a906d1

update more regression data

3042870

spencerkclark and others added 2 commits March 9, 2022 18:51

Ensure dimension coordinates are floats

21d9daa

This ensures that the dummy coordinates we construct during the vertical remapping process have dtype np.float32 from the start. Our coarsening procedure of these coordinates requires they be of a floating point type.

fix regression tests

08d61dd

for some reason these tests fail when run via the `coverage` utility. We don't look at coverage stats much, so I took the expedient option and removed it for this small number of tests.

nbren12 commented Mar 9, 2022

View reviewed changes

nbren12 added 2 commits March 9, 2022 15:32

update dep management docs

8248b59

fix typo in makefile

28bdfa6

spencerkclark approved these changes Mar 10, 2022

View reviewed changes

nbren12 merged commit 00851a3 into master Mar 10, 2022

nbren12 deleted the beam-container branch March 10, 2022 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix dataflow submission #1691

fix dataflow submission #1691

nbren12 commented Mar 8, 2022 •

edited

nbren12 commented Mar 8, 2022 •

edited

nbren12 commented Mar 8, 2022

nbren12 commented Mar 9, 2022

nbren12 left a comment

nbren12 Mar 8, 2022

nbren12 Mar 8, 2022

nbren12 Mar 8, 2022

nbren12 Mar 8, 2022

nbren12 Mar 8, 2022

nbren12 Mar 9, 2022

nbren12 Mar 9, 2022

nbren12 Mar 9, 2022

nbren12 Mar 9, 2022

nbren12 Mar 9, 2022

spencerkclark left a comment

spencerkclark Mar 10, 2022

nbren12 Mar 10, 2022

		testpath==0.4.4
		tf-estimator-nightly==2.8.0.dev2021122109

fix dataflow submission #1691

fix dataflow submission #1691

Conversation

nbren12 commented Mar 8, 2022 • edited

nbren12 commented Mar 8, 2022 • edited

nbren12 commented Mar 8, 2022

nbren12 commented Mar 9, 2022

nbren12 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spencerkclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbren12 commented Mar 8, 2022 •

edited

nbren12 commented Mar 8, 2022 •

edited