CESM support #30

mnlevy1981 · 2022-04-06T19:00:36Z

This branch cleans up the offline driver to make it easy to set up FEISTY runs from netCDF forcing / IC files. Improvements include

Improved interpolation, so we no longer need daily forcing fields
A notebook that generates forcing files from post-processed CESM output -- a future PR to add the post-processing to this repo would be great
Updated default settings to match the FOSI run, with a clear example of how to override the defaults for the previous two test cases
performance improvements to get runtime in line with the matlab performance -- relying on numpy instead of xarray for more computations, using the assume_sorted flag in the scipy interpolation routine, and refactoring how the offline driver stores the model state throughout the run

1. Instead of passing yaml file name and key to _domain_from_netcdf() and _forcing_from_netcdf(), we read the yaml in config_from_netcdf() and pass the dictionary to both routines 2. Added a biomass_init option to constructor for feisty_instance_type -- currently 'constant' is only acceptable value, but this will let us read initial conditions from netCDF as well (needed for FOSI forcing)

The biggest enhancement is that users can now specify a netcdf file containing initial conditions (split into fish_ic and bent_ic variables). I also did a little more performance tuning, relying on numpy instead of xarray in a few more spots in the code.

Modified interpolation routine to use first forcing date when model date is earlier than that and last forcing date when model date is beyond (rather than extrapolating). This new approach to forcing let me remove _forcing_t() from the offline_driver class.

Also included big refactor of matlab-comparison.ipynb to make it easier to add additional tests (e.g. we will want a FOSI_spinup option)

1. Use assume_sorted=True in xr.Dataset.interp() 2. Use .data in more copies so that we're not spending cycles copying metadata

Also added some primative timing info

offline driver now has self._max_output_time_dim (default=365) and _ds is a list of datasets with time dimension no larger than that setting. After running, self.gen_ds() will run xr.concat() to replace it with a single dataset.

The offline driver can now create an initial condition file from the last timestep of the existing state datasets. Also added support to pass the argument max_output_time_dim through config_testcase() and config_from_netcdf(), and am committing an updated matlab-comparison.ipynb to show how to use several new options in the driver.

Used the monthly output from Kristen's post-processed output. Contains raw cells that show how I converted .mat output to initial conditions, but we also have a FOSI_spinup option that should be able to generate new IC files instead.

mnlevy1981 · 2022-04-06T19:01:07Z

This PR will remain a draft until I update the documentation

Needed to add call to testcase.gen_ds()

1. Added missing testcase.gen_ds() calls 2. Updated API in test routines 3. Made sure the test_feisty.py tests were using the same parameters for testing as were used when generating the baseline 4. food_web.get_consumption() returns numpy array, not DataArray Also made sure driver-example.ipynb used old settings (for consistency with prior commits)

When building documentation, we don't want to re-run FOSI-forcing.ipynb

mnlevy1981 · 2022-04-06T21:00:43Z

The only current test failure is

Traceback (most recent call last):
  File "/home/runner/.cache/pre-commit/repopwsd7jk3/py_env-python3.10/bin/black", line 8, in <module>
    sys.exit(patched_main())
  File "/home/runner/.cache/pre-commit/repopwsd7jk3/py_env-python3.10/lib/python3.10/site-packages/black/__init__.py", line 1282, in patched_main
    patch_click()
  File "/home/runner/.cache/pre-commit/repopwsd7jk3/py_env-python3.10/lib/python3.10/site-packages/black/__init__.py", line 1268, in patch_click
    from click import _unicodefun  # type: ignore
ImportError: cannot import name '_unicodefun' from 'click' (/home/runner/.cache/pre-commit/repopwsd7jk3/py_env-python3.10/lib/python3.10/site-packages/click/__init__.py)

It looks like this is due to an incompatibility between black and click, and has been fixed but it's not clear to me how the environment used by pre-commit in linting.yaml is created so I don't know where to either pin click to an old version or update to the newest black

api-driver.rst now references the offline_driver class (previously the simulation class), and api-internal.rst no longer references the compute_benthic_biomass_update() function since that was removed a while ago

mnlevy1981 · 2022-04-06T22:03:48Z

Assuming the testing passes (except the failure due to black / click), this is ready to be merged and I believe the documentation will be updated once this is available on main.

1. Add matlab-comparison notebook to the docs 2. Update matlab-comparison to include more details on what is in the notebook 3. Update various environments so hopefully everything builds correctly

New black version flagged formatting issue that was previously okay

mnlevy1981 · 2022-04-07T19:29:09Z

It looks like this is due to an incompatibility between black and click, and has been fixed but it's not clear to me how the environment used by pre-commit in linting.yaml

I ran pre-commit autoupdate and that brought in newer versions of these packages, so we're passing CI again

matt-long

Looks great!

matt-long · 2022-04-08T17:12:58Z

feisty/core/ecosystem.py

@@ -274,8 +276,9 @@ def is_small(self):
 class zooplankton_type(object):
    """Data structure containing zooplankton parameters."""

-    def __init__(self, name, **kwargs):
+    def __init__(self, glob_id, name, **kwargs):


can we change glob_id to global_id or group_id?

matt-long · 2022-04-08T17:16:34Z

feisty/core/interface.py

@@ -53,6 +55,7 @@ def __init__(
        settings_dict={},
        fish_ic_data=None,
        benthic_prey_ic_data=None,
+        biomass_init='constant',


ensure that this plays friendly with fish_ic_data

another option:

biomass_init --> biomass_init_file, default None

if biomass_init_file is not None: ds_ic = xr.open... assert varnames for all taxa in ds_ic # use it a else: # use fish_ic_data # use benthic_ic_data ....

matt-long · 2022-04-08T17:20:36Z

feisty/core/interface.py

@@ -53,6 +55,7 @@ def __init__(
        settings_dict={},
        fish_ic_data=None,


Suggested change

fish_ic_data=None,

fish_ic_data=1e-5,

benthic_ic_data=2e-3,

matt-long · 2022-04-08T17:24:02Z

feisty/core/interface.py

+        self.tendency_data.predation_rate[
+            :, :
+        ] = self.tendency_data.predation_flux.data / np.maximum(
+            self.biomass.data[self.ndx_fish, :], 1e-300


should 1e-300 be replaced by eps?

Perhaps we should also introduce a parameter for minimum biomass?

matt-long · 2022-04-08T17:26:12Z

feisty/core/process.py

@@ -229,24 +227,28 @@ def compute_rescale_zoo_consumption(
    for zoo_i in food_web.zoo_names:

        link_ndx = food_web.prey_link_ndx[zoo_i]
-        biomass_zoo_pred = food_web._get_biomass_zoo_pred(biomass, zoo_i)
+        biomass_zoo_pred = np.maximum(food_web._get_biomass_zoo_pred(biomass, zoo_i).data, 1e-300)


use constants.eps

matt-long · 2022-04-08T17:31:45Z

feisty/offline_driver.py

@@ -110,6 +109,10 @@ def __init__(
        settings_in={},
        fish_ic_data=None,
        benthic_prey_ic_data=None,
+        biomass_init='constant',
+        allow_negative_forcing=False,
+        diagnostic_names=[],


do we want to provide a utility to query available diags? Or have a minimal set of defaults?

matt-long · 2022-04-08T17:33:27Z

feisty/offline_driver.py

-            'consumption_rate_max_pred',
-            'consumption_rate_link',
-        ]
+        self._diagnostic_names = diagnostic_names


change _diagnostic_names to _add_diagnostic_names?

matt-long · 2022-04-08T17:35:24Z

feisty/offline_driver.py

+        """
+        self._ds = []
+        start_dates = [self.start_date]
+        days_per_year = [np.min([nt, self._max_output_time_dim])]


days_per_year ---> time_levs_per_ds

matt-long · 2022-04-08T17:46:28Z

feisty/offline_driver.py

        """Return the feisty time tendency."""
-        gcm_data_t = self._forcing_t(t)
+        gcm_data_t = self.forcing.interp(time=forcing_t, assume_sorted=True)
+        if not self.allow_negative_forcing:


might this logic belong in feisty_instance?

matt-long · 2022-04-08T17:50:16Z

feisty/offline_driver.py

+        else:
+            print(f'Finished _solve at {time.strftime("%H:%M:%S")}')
+
+    def create_ic_file_from_final_state(self, ic_file, overwrite=False):


Suggested change

def create_ic_file_from_final_state(self, ic_file, overwrite=False):

def create_restart_file(self, ic_file, overwrite=False):

Moved some functions into new utils module, and then flake helped me clean up bugs in those functions that hadn't been caught in the notebook Also removed prettier from pre-commit, because it wasn't playing nice on cheyenne (CalledProcessError with node because GLIBC versions weren't found)

Prettier is back in the pre-commit suite, and the deafult_language_version option lets it install without the GLIBC errors

The forcing_time array wasn't being set properly for non-cyclic forcing Also cleaned up matlab-comparison.ipynb following a code walkthrough: 1. rename "testcase" -> "fiesty_driver" 2. set matlab_script in one of the top cells (easy for others to change)

Instead of a cell that prints matlab_script, add that output to the cell that calls .run(); also, FOSI_cesm should use initial conditions spun up in python rather than matlab

1. Renamed glob_id -> group_ind 2. Clean up how we initialize biomass (biomass_init_file = None => fallback to fish_ic_data and benthic_prey_ic_data) 3. 1e-300 -> constants.eps 4. days_per_year -> time_levs_per_ds 5. create_ic_file_from_final_state -> create_restart_file

mnlevy1981 · 2022-04-27T21:52:19Z

071770d cleans up the straight-forward comments @matt-long made, but the following are NOT yet addressed:

better logic surrounding what diagnostics are included in the output
moving forcing logic into feisty_instance

mnlevy1981 · 2022-04-27T21:52:45Z

Also still working on parallelizing matlab-comparison.ipynb

domain.py and ecosystem.py no longer use module (global) variables; instead, the interface class initializes self.domain_params and self.ecosys_params and passes those objects to functions that rely on the previously-global variables.

mnlevy1981 · 2022-04-29T21:50:55Z

ea12b57 addresses #20 because we can't parallelize (part of #21) with global variables in some of the modules

When I changed glob_id -> group_ind, I missed all the occurences in interface.py. I also need to add docs/__init__.py to the repository so that matlab-examples.ipynb can import functions from utils.py

When dimensions don't match in interface.py, include expected / received dims in the exception

Needed to rename time -> forcing_time in the forcing dataset so that dask wouldn't get confused when the template for the output had a time dimension that didn't match what was in forcing

also cleaned up some of the driver code when testing uncovered small bugs

Just a little more re-organizing

Also removed python3.9 from environment because it was overriding the python_version specified in the CI matrix. I'll test the notebooks with 3.10 again, and if dask is still not playing nice I'll figure out a good way to have users downgrade to 3.9

also updated docs/_config.yml to hopefully pass CI

Remove the "with dask" matlab comparison notebook and rename the "with dask2" comparison notebook

Refactored offline_driver to combine config and run into a single function, which can then use xr.map_blocks() to wrap a call to a function that actually creates the offline_driver object and steps through time in parallel

Contains a notebook for processing FOSI, as well as a .py script (that uses dask-mpi) and a shell script that submits the .py script to casper Also added the "pip install -e" to environment.yml (along with dask-mpi and its dependencies), and hopefully cleaned up the CI without breaking anything

also added a README to describe the variables (the prettier pre-commit check made it hard to do this with comments in the YAML itself)

FOSI_cesm.ipynb has FOSI-specific parameters hard-coded, but with the .py script using YAML it makes sense to give it a more generic name.

Re-organize / add comments to yaml file; only write output file in the python driver if output file is provided in yaml

1. Support for reading raw POP output instead of requiring post-processing 2. Support for reading multiple files when generating forcing dataset (e.g. provide a list of multiple POP time series files) 3. Support for mapping output back to the POP grid -- read in POP data on nlat, nlon grid -- map to single horizontal dimension (X) -- run FEISTY -- map output back to nlat, nlon grid

Also, spelled "high" correctly in highres_companion.yaml file name and updated a notebook to only provide 2D output if requested in the YAML file

mnlevy1981 · 2022-09-09T21:51:16Z

@matt-long I think this is ready for review. The examples/ directory shows how to set up YAML files to point FEISTY to forcing data, and I'm happy to walk through that with you.

Looking back at previous comments, I'm not sure I ever addressed this:

071770d cleans up the straight-forward comments @matt-long made, but the following are NOT yet addressed:

better logic surrounding what diagnostics are included in the output

moving forcing logic into feisty_instance

mnlevy1981 added 9 commits March 22, 2022 15:37

Change default_settings to match FOSI

17a4b3b

Also included big refactor of matlab-comparison.ipynb to make it easier to add additional tests (e.g. we will want a FOSI_spinup option)

Some performance improvements

3d7ea56

1. Use assume_sorted=True in xr.Dataset.interp() 2. Use .data in more copies so that we're not spending cycles copying metadata

Reduce default output in driver

9b7bb61

Also added some primative timing info

Break up _ds into smaller chunks

e3820ed

offline driver now has self._max_output_time_dim (default=365) and _ds is a list of datasets with time dimension no larger than that setting. After running, self.gen_ds() will run xr.concat() to replace it with a single dataset.

Add notebook used to generate FOSI forcing

fb1186b

Used the monthly output from Kristen's post-processed output. Contains raw cells that show how I converted .mat output to initial conditions, but we also have a FOSI_spinup option that should be able to generate new IC files instead.

mnlevy1981 added 3 commits April 6, 2022 13:31

Update driver-example with new API

f571092

Needed to add call to testcase.gen_ds()

Ignore new FOSI-forcing notebook

ebeab47

When building documentation, we don't want to re-run FOSI-forcing.ipynb

Update documentation

3b069a6

api-driver.rst now references the offline_driver class (previously the simulation class), and api-internal.rst no longer references the compute_benthic_biomass_update() function since that was removed a while ago

mnlevy1981 marked this pull request as ready for review April 6, 2022 22:03

mnlevy1981 requested a review from matt-long April 6, 2022 22:04

mnlevy1981 added 2 commits April 7, 2022 11:59

Documentation updates

daf8cc9

1. Add matlab-comparison notebook to the docs 2. Update matlab-comparison to include more details on what is in the notebook 3. Update various environments so hopefully everything builds correctly

Formatting update to pass CI

a679f82

New black version flagged formatting issue that was previously okay

matt-long mentioned this pull request Apr 8, 2022

single output memory buffer #31

Open

matt-long reviewed Apr 8, 2022

View reviewed changes

mnlevy1981 added 5 commits April 21, 2022 10:46

Better fix for pre-commit issue with prettier

d58d7c3

Prettier is back in the pre-commit suite, and the deafult_language_version option lets it install without the GLIBC errors

Fix bug in offline_driver

d0aea40

The forcing_time array wasn't being set properly for non-cyclic forcing Also cleaned up matlab-comparison.ipynb following a code walkthrough: 1. rename "testcase" -> "fiesty_driver" 2. set matlab_script in one of the top cells (easy for others to change)

Clean up output in matlab-comparison

ad0d76f

Instead of a cell that prints matlab_script, add that output to the cell that calls .run(); also, FOSI_cesm should use initial conditions spun up in python rather than matlab

Move global parameters into new classes

ea12b57

domain.py and ecosystem.py no longer use module (global) variables; instead, the interface class initializes self.domain_params and self.ecosys_params and passes those objects to functions that rely on the previously-global variables.

This was linked to issues Apr 29, 2022

refactor process module #3

Closed

complete verification relative to Matlab version #19

Closed

thread safety #20

Closed

improve performance #21

Closed

mnlevy1981 added 15 commits April 29, 2022 16:17

Missed some previous code clean-up

7adfc54

When I changed glob_id -> group_ind, I missed all the occurences in interface.py. I also need to add docs/__init__.py to the repository so that matlab-examples.ipynb can import functions from utils.py

Improve error messages

bb28761

When dimensions don't match in interface.py, include expected / received dims in the exception

Add support for running with dask

8d91d5c

Needed to rename time -> forcing_time in the forcing dataset so that dask wouldn't get confused when the template for the output had a time dimension that didn't match what was in forcing

Two new notebooks showing parallelization

6c3ec00

also cleaned up some of the driver code when testing uncovered small bugs

More notebook cleanup

24c67cf

Just a little more re-organizing

Fix failing pytests

d9d75f7

Also removed python3.9 from environment because it was overriding the python_version specified in the CI matrix. I'll test the notebooks with 3.10 again, and if dask is still not playing nice I'll figure out a good way to have users downgrade to 3.9

Tested with 3.10 and dask is happy now

788793d

also updated docs/_config.yml to hopefully pass CI

Clean up some of the new notebooks

ec9e6d8

Remove the "with dask" matlab comparison notebook and rename the "with dask2" comparison notebook

Consistent approach to parallel / serial driver

8d309de

Refactored offline_driver to combine config and run into a single function, which can then use xr.map_blocks() to wrap a call to a function that actually creates the offline_driver object and steps through time in parallel

Move parameters from python script -> YAML

7bd0a8f

also added a README to describe the variables (the prettier pre-commit check made it hard to do this with comments in the YAML itself)

Rename FOSI_cesm.py -> FEISTY_driver.py

4029fc7

FOSI_cesm.ipynb has FOSI-specific parameters hard-coded, but with the .py script using YAML it makes sense to give it a more generic name.

Clean up params.yaml

85a5cce

Re-organize / add comments to yaml file; only write output file in the python driver if output file is provided in yaml

driver.sh lets you specify YAML file

5a17557

Also, spelled "high" correctly in highres_companion.yaml file name and updated a notebook to only provide 2D output if requested in the YAML file

mnlevy1981 requested a review from matt-long September 9, 2022 21:54

mnlevy1981 merged commit e411e79 into marbl-ecosys:main Sep 12, 2022

mnlevy1981 deleted the cesm_support branch January 17, 2023 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CESM support #30

CESM support #30

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 7, 2022

matt-long left a comment

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

matt-long Apr 8, 2022

mnlevy1981 commented Apr 27, 2022

mnlevy1981 commented Apr 27, 2022

mnlevy1981 commented Apr 29, 2022

mnlevy1981 commented Sep 9, 2022 •

edited

Loading

		@@ -53,6 +55,7 @@ def __init__(
		settings_dict={},
		fish_ic_data=None,

	def create_ic_file_from_final_state(self, ic_file, overwrite=False):
	def create_restart_file(self, ic_file, overwrite=False):

CESM support #30

CESM support #30

Conversation

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 6, 2022

mnlevy1981 commented Apr 7, 2022

matt-long 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

Choose a reason for hiding this comment

mnlevy1981 commented Apr 27, 2022

mnlevy1981 commented Apr 27, 2022

mnlevy1981 commented Apr 29, 2022

mnlevy1981 commented Sep 9, 2022 • edited Loading

mnlevy1981 commented Sep 9, 2022 •

edited

Loading