Merge pull request #242 from amoodie/maintain

some maintenance and some features (non-breaking)

docs/source/guides/advanced_configuration_guide.inc

@@ -165,7 +165,8 @@ We then can initialize our new model type, and see that this model has all of th
 
 Hooks are methods in the model sequence that do nothing by default, but can be augmented to provide arbitrary desired behavior in the model.
 Hooks have been integrated throughout the model initialization and update sequences, to allow the users to achieve complex behavior at various stages of the model sequence.
-For example, ``hook_solve_water_and_sediment_timestep`` is a hook which occurs immediately before the model :obj:`solve_water_and_sediment_timestep` method.
+For example, ``hook_solve_water_and_sediment_timestep`` is a hook which occurs *immediately before* the model :obj:`solve_water_and_sediment_timestep` method.
+The standard is for a `hook` to describe the function name that it precedes.
 
 To utilize the hooks, we simply define a method in our subclass with the name corresponding to the hook we want to augment to achieve the desired behavior.
 For example, to change the behavior of the subsidence field to vary randomly in magnitude on each iteration:
@@ -218,7 +219,9 @@ Now, on every iteration of the model, our hooked method will be called immediate
 
 This is a somewhat contrived example to give you a sense of how you can implement changes to the model to achieve desired behavior. 
 
-A complete :doc:`list of model hooks is available here </reference/model/model_hooks>`, but model hooks all follow the convention of beginning with the prefix `hook_` and include the name of the method they *immediately precede* in the model; for example `hook_run_water_iteration` is called immediately before `run_water_iteration` is called.
+A complete :doc:`list of model hooks is available here </reference/model/model_hooks>`, but the standard is for model hooks to follow a convention of beginning with the prefix `hook_` and include the name of the method they *immediately precede* in the model; for example `hook_run_water_iteration` is called immediately before `run_water_iteration` is called.
+There are a few additional hooks integrated into the model that occur *after* logical collections of functions, where a developer is likely to want to integrate a change in behavior.
+These hooks follow the convention name `hook_after_` and then the name of the function they follow; two examples are `hook_after_route_water` and `hook_after_route_sediment`.
 
 .. note::
 
@@ -231,6 +234,7 @@ A complete :doc:`list of model hooks is available here </reference/model/model_h
 There may be cases where hooks are insufficient for the modifications you need to make to the model.
 In this case, you should subclass the model, as depicted above, and re-implement entire methods, as necessary, by copying code from the package to your own subclass.
 However, for stability and forwards-compatibility, you should try to minimize copied code as much as possible.
+If you identify a location in the model framework where you think adding a hook (for example, another `hook_after_` method), please file an issue on the GitHub and we will be happy to consider.
 
 
 Subclassing examples

docs/source/reference/model/model_hooks.rst

@@ -22,3 +22,5 @@ A complete list of hooks in the model follows:
     , :obj:`~hook_tools.hook_topo_diffusion`
     , :obj:`~hook_tools.hook_route_all_mud_parcels`
     , :obj:`~hook_tools.hook_compute_sand_frac`
+    , :obj:`~hook_tools.hook_after_route_water`
+    , :obj:`~hook_tools.hook_after_route_sediment`

docs/source/reference/shared_tools/index.rst

@@ -19,6 +19,7 @@ Shared functions
 This module defines several functions that are used throughout the model, and so are organized here for convenience.
 
 .. autofunction:: get_random_uniform
+.. autofunction:: get_inlet_weights
 .. autofunction:: get_start_indices
 .. autofunction:: get_steps
 .. autofunction:: random_pick

pyDeltaRCM/hook_tools.py

@@ -246,3 +246,21 @@ def hook_init_output_file(self):
 
         """
         pass
+
+    def hook_after_route_water(self):
+        """Hook called *after* :obj:`~pyDeltaRCM.water_tools.water_tools.route_water`.
+
+        Unlike the standard model hooks, this hook is called *after* water
+        routing has completed with the
+        method :obj:`~pyDeltaRCM.water_tools.water_tools.route_water`
+        """
+        pass
+
+    def hook_after_route_sediment(self):
+        """Hook called *after* :obj:`~pyDeltaRCM.sed_tools.sed_tools.route_sediment`.
+
+        Unlike the standard model hooks, this hook is called *after* sed
+        routing has completed with the
+        method :obj:`~pyDeltaRCM.sed_tools.sed_tools.route_sediment`
+        """
+        pass

pyDeltaRCM/init_tools.py

@@ -587,7 +587,8 @@ def init_output_file(self):
             if (os.path.exists(file_path)) and \
                     (self._clobber_netcdf is False):
                 raise FileExistsError(
-                    'Existing NetCDF4 output file in target output location.')
+                    'Existing NetCDF4 output file in target output location: '
+                    '{file_path}'.format(file_path=file_path))
             elif (os.path.exists(file_path)) and \
                     (self._clobber_netcdf is True):
                 _msg = 'Replacing existing netCDF file'
@@ -659,17 +660,18 @@ def _create_meta_variable(varname, varvalue, varunits,
 
             self.output_netcdf.createGroup('meta')
             for _val in self._save_var_list['meta'].keys():
-                # time-varying initialize w/ None value, fixed use attribute
+                # time-varying initialize w/ None value
                 if (self._save_var_list['meta'][_val][0] is None):
                     _create_meta_variable(
                         _val, self._save_var_list['meta'][_val][0],
                         self._save_var_list['meta'][_val][1],
                         self._save_var_list['meta'][_val][2],
                         self._save_var_list['meta'][_val][3])
+                # for scalars, get the attribute and store it
                 else:
                     _create_meta_variable(
-                        _val, getattr(self,
-                                      self._save_var_list['meta'][_val][0]),
+                        _val, getattr(
+                            self, self._save_var_list['meta'][_val][0]),
                         self._save_var_list['meta'][_val][1],
                         self._save_var_list['meta'][_val][2],
                         self._save_var_list['meta'][_val][3])

pyDeltaRCM/iteration_tools.py

@@ -41,10 +41,12 @@ def solve_water_and_sediment_timestep(self):
         # water iterations
         self.hook_route_water()
         self.route_water()
+        self.hook_after_route_water()
 
         # sediment iteration
         self.hook_route_sediment()
         self.route_sediment()
+        self.hook_after_route_sediment()
 
     def run_one_timestep(self):
         """Deprecated, since v1.3.1. Use :obj:`solve_water_and_sediment_timestep`."""

pyDeltaRCM/sed_tools.py

@@ -74,6 +74,23 @@ def init_sediment_iteration(self):
         self.Vp_dep_sand[:] = 0
         self.Vp_dep_mud[:] = 0
 
+    def get_inlet_weights_sediment(self, **kwargs):
+        """Get weight for inlet cells for sediment parcels.
+
+        This method determines the *weights* describing which inlet cells
+        sediment parcels are fed into the domain (where the cells are
+        `self.inlet`).
+        Internally, :obj:`pyDeltaRCM.shared_tools._get_inlet_weights` is
+        called, returning a balanced weighting across all inlet cells.
+
+        .. note::
+
+            Reimplement this method in custom subclasses as needed. Function
+            is passed a string argument `parcel_type`, with information about
+            the parcel being routed for convenience.
+        """
+        return shared_tools.get_inlet_weights(self.inlet)
+
     def route_all_sand_parcels(self):
         """Route sand parcels; topo diffusion.
 
@@ -100,7 +117,7 @@ def route_all_sand_parcels(self):
         self.log_info(_msg, verbosity=2)
 
         num_starts = int(self._Np_sed * self._f_bedload)
-        inlet_weights = np.ones_like(self.inlet)
+        inlet_weights = self.get_inlet_weights_sediment(parcel_type='sand')
         start_indices = shared_tools.get_start_indices(self.inlet,
                                                        inlet_weights,
                                                        num_starts)
@@ -155,7 +172,7 @@ def route_all_mud_parcels(self):
         self.log_info(_msg, verbosity=2)
 
         num_starts = int(self._Np_sed * (1 - self._f_bedload))
-        inlet_weights = np.ones_like(self.inlet)
+        inlet_weights = self.get_inlet_weights_sediment(parcel_type='mud')
         start_indices = shared_tools.get_start_indices(self.inlet,
                                                        inlet_weights,
                                                        num_starts)

pyDeltaRCM/shared_tools.py

@@ -146,6 +146,30 @@ def get_random_uniform(limit):
     return np.random.uniform(0, limit)
 
 
+def get_inlet_weights(inlet):
+    """Get weight for inlet parcels.
+
+    This method is called at the top of water and sediment routing steps by
+    default, as implemented in the methods
+    :obj:`~pyDeltaRCM.water_tools.water_tools.get_inlet_weights_water` and
+    :obj:`~pyDeltaRCM.water_tools.water_tools.get_inlet_weights_sediment`.
+
+    The function determines the *weights* describing which inlet cells parcels
+    are fed into the domain. As implemented by default, returns "ones" for
+    all inlet cell locations, so that the input sediment source is balanced
+    across all inlet cells. The returned weight array is typically passed
+    to :obj:`get_start_indices` to determine the locations to feed parcels
+    into the domain based on the weights determined by this function.
+
+    Parameters
+    ----------
+    inlet : :obj:`ndarray` of int
+        Integer array specifying the inlet cell indices.
+    """
+    inlet_weights = np.ones_like(inlet)
+    return inlet_weights
+
+
 @njit
 def get_start_indices(inlet, inlet_weights, num_starts):
     """Get start indices.

pyDeltaRCM/water_tools.py

@@ -62,6 +62,21 @@ def init_water_iteration(self):
         self.pad_depth = np.pad(self.depth, 1, 'edge')
         self.pad_cell_type = np.pad(self.cell_type, 1, 'edge')
 
+    def get_inlet_weights_water(self, **kwargs):
+        """Get weight for inlet cells for water parcels.
+
+        This method determines the *weights* describing which inlet cells
+        water parcels are fed into the domain (where the cells are
+        `self.inlet`).
+        Internally, :obj:`pyDeltaRCM.shared_tools._get_inlet_weights` is
+        called, returning a balanced weighting across all inlet cells.
+
+        .. note::
+
+            Reimplement this method in custom subclasses as needed.
+        """
+        return shared_tools.get_inlet_weights(self.inlet)
+
     def run_water_iteration(self):
         """Run a single iteration of travel paths for all water parcels.
 
@@ -81,7 +96,7 @@ def run_water_iteration(self):
         self.log_info(_msg, verbosity=2)
 
         # configure the starting indices for each parcel
-        inlet_weights = np.ones_like(self.inlet)
+        inlet_weights = self.get_inlet_weights_water()
         start_indices = shared_tools.get_start_indices(self.inlet,
                                                        inlet_weights,
                                                        self._Np_water)
@@ -320,7 +335,7 @@ def check_size_of_indices_matrix(self, it):
             self.log_info(_msg, verbosity=2)
 
             indices_blank = np.zeros(
-                (np.int(self._Np_water), np.int(self.stepmax / 4)), dtype=int)
+                (int(self._Np_water), int(self.stepmax / 4)), dtype=int)
 
             self.free_surf_walk_inds = np.hstack((self.free_surf_walk_inds, indices_blank))
 
@@ -811,7 +826,7 @@ def _check_for_loops(free_surf_walk_inds, new_inds, _step,
         the inlet domain edge.
 
     stage_above_SL
-        Water surface elevation minuns the domain sea level.
+        Water surface elevation minus the domain sea level.
 
     Returns
     -------

tests/integration/test_checkpointing.py

@@ -187,8 +187,8 @@ def test_checkpoint_nc(self, tmp_path):
         assert np.all(output['meta']['cell_type'][:] == resumeModel.cell_type)
         assert output['meta']['H_SL'][-1].data == resumeModel.H_SL
         assert output['meta']['f_bedload'][-1].data == resumeModel.f_bedload
-        assert pytest.approx(float(output['meta']['C0_percent'][-1].data) ==
-                             resumeModel.C0_percent)
+        C0_from_file = float(output['meta']['C0_percent'][-1].data)
+        assert pytest.approx(C0_from_file) == resumeModel.C0_percent
         assert output['meta']['u0'][-1].data == resumeModel.u0
 
         # checkpoint interval aligns w/ timestep dt so these should match
@@ -696,7 +696,7 @@ def test_load_checkpoint_with_open_netcdf(self, tmp_path):
         assert np.all(_opened['eta'][:].data == _new['eta'][0, :, :].data)
         # random field should be saved in the new netCDF file
         # some rounding/truncation happens in the netCDF so we use approx
-        assert pytest.approx(_rand_field == _new['eta'][1, :, :].data)
+        assert pytest.approx(_rand_field) == _new['eta'][1, :, :].data
 
     @pytest.mark.skipif(
         platform.system() != 'Windows',