Skip to content

Commit

Permalink
Bug: protect parameters as readonly np.arrays and MappingProxyTypes i…
Browse files Browse the repository at this point in the history
…nstead of dicts
  • Loading branch information
jmccreight committed May 26, 2023
1 parent dd99eb7 commit 9c92dab
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 30 deletions.
6 changes: 5 additions & 1 deletion autotest/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,9 @@ def test_parameter_json(domain, tmp_path):
assert json_file.exists()

params_from_json = PrmsParameters.load_from_json(json_file)
np.testing.assert_equal(parameters.data, params_from_json.data)
assert parameters.data.keys() == params_from_json.data.keys()
for kk in parameters.data.keys():
np.testing.assert_equal(
dict(parameters.data[kk]), dict(params_from_json.data[kk])
)
return
8 changes: 6 additions & 2 deletions pywatershed/base/control.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""The control class."""
import datetime
from types import MappingProxyType

import numpy as np

Expand Down Expand Up @@ -70,7 +71,8 @@ def __init__(
self.config = config
self.params = params
if params is not None:
self.params.dims["ntime"] = self.n_times
# this should be a super private method on parameters
self.edit_n_time_steps(self.n_times)

self.meta = meta
# This will have the time dimension name
Expand Down Expand Up @@ -237,5 +239,7 @@ def edit_n_time_steps(self, new_n_time_steps: int):
self._end_time = (
self._start_time + (self._n_times - 1) * self._time_step
)
self.params.dims["ntime"] = self._n_times
self.params._dims = MappingProxyType(
self.params.dims | {"ntime": self._n_times}
)
return
16 changes: 16 additions & 0 deletions pywatershed/base/parameters.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from types import MappingProxyType
from typing import Union

import numpy as np
Expand Down Expand Up @@ -32,6 +33,10 @@ def __init__(
encoding=encoding,
validate=validate,
)

for kk in self._keys():
self[f"_{kk}"] = _set_dict_read_only(self[kk])

return

@property
Expand Down Expand Up @@ -61,3 +66,14 @@ def get_dim_values(
return self.dims[keys]
else:
return {kk: self.dims[kk] for kk in keys}


def _set_dict_read_only(dd: dict):
for kk, vv in dd.items():
if isinstance(vv, dict):
_set_dict_read_only(vv)
dd[kk] = MappingProxyType(vv)
elif isinstance(vv, np.ndarray):
vv.flags.writeable = False

return MappingProxyType(dd)
18 changes: 9 additions & 9 deletions pywatershed/hydrology/PRMSChannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,14 @@ def _initialize_channel_data(self) -> None:
"""Initialize internal variables from raw channel data"""

# convert prms data to zero-based
self.hru_segment -= 1
self.tosegment -= 1
self._hru_segment = self.hru_segment - 1
self._tosegment = self.tosegment - 1

# calculate connectivity
self._outflow_mask = np.full((len(self.tosegment)), False)
self._outflow_mask = np.full((len(self._tosegment)), False)
connectivity = []
for iseg in range(self.nsegment):
tosegment = self.tosegment[iseg]
tosegment = self._tosegment[iseg]
if tosegment < 0:
self._outflow_mask[iseg] = True
continue
Expand Down Expand Up @@ -326,7 +326,7 @@ def _initialize_channel_data(self) -> None:

# initialize internal self_inflow variable
for iseg in range(self.nsegment):
jseg = self.tosegment[iseg]
jseg = self._tosegment[iseg]
if jseg < 0:
continue
self._seg_inflow[jseg] = self.seg_outflow[iseg]
Expand Down Expand Up @@ -361,7 +361,7 @@ def _calculate(self, simulation_time: float) -> None:
# calculate lateral flow term
self.seg_lateral_inflow[:] = 0.0
for ihru in range(self.nhru):
iseg = self.hru_segment[ihru]
iseg = self._hru_segment[ihru]
if iseg < 0:
# This is bad, selective handling of fluxes is not cool,
# mass is being discarded in a way that has to be coordinated
Expand Down Expand Up @@ -445,7 +445,7 @@ def _calculate(self, simulation_time: float) -> None:
self._seg_current_sum[:],
) = self._muskingum_mann_numba(
self._segment_order,
self.tosegment,
self._tosegment,
self.seg_lateral_inflow,
self._seg_inflow0,
self._outflow_ts,
Expand All @@ -467,7 +467,7 @@ def _calculate(self, simulation_time: float) -> None:
self._seg_current_sum[:],
) = self._muskingum_mann_numpy(
self._segment_order,
self.tosegment,
self._tosegment,
self.seg_lateral_inflow,
self._seg_inflow0,
self._outflow_ts,
Expand All @@ -489,7 +489,7 @@ def _calculate(self, simulation_time: float) -> None:
self._seg_current_sum[:],
) = _calculate_fortran(
self._segment_order,
self.tosegment,
self._tosegment,
self.seg_lateral_inflow,
self._seg_inflow0,
self._outflow_ts,
Expand Down
13 changes: 10 additions & 3 deletions pywatershed/hydrology/PRMSGroundwater.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ def __init__(

self._set_inputs(locals())
self._set_budget(budget_type)

if calc_method == "numba":
# read-only arrays dont have numba signatures
self._hru_area = self.hru_area.copy()
self._gwflow_coef = self.gwflow_coef.copy()
self._gwsink_coef = self.gwsink_coef.copy()

return

@staticmethod
Expand Down Expand Up @@ -178,13 +185,13 @@ def _calculate(self, simulation_time):
self.gwres_stor_change[:],
self.gwres_flow_vol[:],
) = self._calculate_numba(
self.hru_area,
self._hru_area,
self.soil_to_gw,
self.ssr_to_gw,
self.dprst_seep_hru,
self.gwres_stor,
self.gwflow_coef,
self.gwsink_coef,
self._gwflow_coef,
self._gwsink_coef,
self.gwres_stor_old,
self.control.params.hru_in_to_cf,
)
Expand Down
2 changes: 1 addition & 1 deletion pywatershed/hydrology/PRMSSnow.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ def _calculate_numpy(
None
"""

canopy_covden = covden_win
canopy_covden = covden_win.copy()
wh_transp_on = np.where(transp_on)
canopy_covden[wh_transp_on] = covden_sum[wh_transp_on]

Expand Down
31 changes: 17 additions & 14 deletions pywatershed/hydrology/PRMSSoilzone.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,12 @@ def _set_initial_conditions(self):
(self.hru_type == HruType.INACTIVE.value)
| (self.hru_type == HruType.LAKE.value)
)
self.sat_threshold[wh_inactive_or_lake] = zero
self._sat_threshold = self.sat_threshold.copy()
self._sat_threshold[wh_inactive_or_lake] = zero
# edit a param
wh_not_land = np.where(self.hru_type != HruType.LAND.value)
self.pref_flow_den[wh_not_land] = zero
self._pref_flow_den = self.pref_flow_den.copy()
self._pref_flow_den[wh_not_land] = zero

# variables
if self.control.config["init_vars_from_file"] in [0, 2, 5]:
Expand All @@ -270,7 +272,7 @@ def _set_initial_conditions(self):

# ssres_stor
if self.control.config["init_vars_from_file"] in [0, 2, 5]:
self.ssres_stor = self.ssstor_init_frac * self.sat_threshold
self.ssres_stor = self.ssstor_init_frac * self._sat_threshold
wh_inactive_or_lake = np.where(
(self.hru_type == HruType.INACTIVE.value)
| (self.hru_type == HruType.LAKE.value)
Expand Down Expand Up @@ -312,29 +314,30 @@ def _set_initial_conditions(self):
self.soil_rechr > self.soil_moist, self.soil_moist, self.soil_rechr
)
self.ssres_stor = np.where(
self.ssres_stor > self.sat_threshold,
self.sat_threshold,
self.ssres_stor > self._sat_threshold,
self._sat_threshold,
self.ssres_stor,
)

# <
# need to set on swale_limit self? move to variables?
self._swale_limit = np.full(self.nhru, zero, "float64")
wh_swale = np.where(self.hru_type == HruType.SWALE.value)
self._swale_limit[wh_swale] = 3.0 * self.sat_threshold[wh_swale]
self._swale_limit[wh_swale] = 3.0 * self._sat_threshold[wh_swale]

self.pref_flow_thrsh[wh_swale] = self.sat_threshold[wh_swale]
self.pref_flow_thrsh[wh_swale] = self._sat_threshold[wh_swale]
wh_land = np.where(self.hru_type == HruType.LAND.value)
self.pref_flow_thrsh[wh_land] = self.sat_threshold[wh_land] * (
one - self.pref_flow_den[wh_land]
self.pref_flow_thrsh[wh_land] = self._sat_threshold[wh_land] * (
one - self._pref_flow_den[wh_land]
)
self.pref_flow_max[wh_land] = (
self.sat_threshold[wh_land] - self.pref_flow_thrsh[wh_land]
self._sat_threshold[wh_land] - self.pref_flow_thrsh[wh_land]
)

# Need to set pref_flow_flag on self? or add to variables?
wh_land_and_prf_den = np.where(
(self.hru_type == HruType.LAND.value) & (self.pref_flow_den > zero)
(self.hru_type == HruType.LAND.value)
& (self._pref_flow_den > zero)
)
self._pref_flow_flag = np.full(self.nhru, False, dtype=int)
self._pref_flow_flag[wh_land_and_prf_den] = True
Expand Down Expand Up @@ -368,7 +371,7 @@ def _set_initial_conditions(self):
self._soil2gw_flag[wh_soil2gwmax] = True

self.soil_zone_max = (
self.sat_threshold + self.soil_moist_max * self.hru_frac_perv
self._sat_threshold + self.soil_moist_max * self.hru_frac_perv
)
self.soil_moist_tot = (
self.ssres_stor + self.soil_moist * self.hru_frac_perv
Expand Down Expand Up @@ -495,7 +498,7 @@ def _calculate(self, simulation_time):
pref_flow_stor_prev=self.pref_flow_stor_prev,
pref_flow_thrsh=self.pref_flow_thrsh,
recharge=self.recharge,
sat_threshold=self.sat_threshold,
sat_threshold=self._sat_threshold,
slow_flow=self.slow_flow,
slow_stor=self.slow_stor,
slow_stor_change=self.slow_stor_change,
Expand Down Expand Up @@ -618,7 +621,7 @@ def _calculate(self, simulation_time):
pref_flow_stor_prev=self.pref_flow_stor_prev,
pref_flow_thrsh=self.pref_flow_thrsh,
recharge=self.recharge,
sat_threshold=self.sat_threshold,
sat_threshold=self._sat_threshold,
slow_flow=self.slow_flow,
slow_stor=self.slow_stor,
slow_stor_change=self.slow_stor_change,
Expand Down

0 comments on commit 9c92dab

Please sign in to comment.