Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

Merged
merged 59 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
f67d3bc
Improving errors in another set of `gempyor` files
emprzy Nov 1, 2024
141a627
Merge branch 'HopkinsIDD:main' into gempyor_error_improvments_part2
emprzy Nov 4, 2024
1fe2d4b
Merge branch 'main' into gempyor_error_improvements_part2
emprzy Nov 8, 2024
90b3618
Applied `black` linting to this branch
emprzy Nov 8, 2024
1934338
Merge branch 'main' into gempyor_error_improvements_part2
emprzy Nov 8, 2024
a6c3585
Updating a RegEx in `test_model_info.py`
emprzy Nov 8, 2024
e2a87c3
`black` linting and RegEx alteration
emprzy Nov 8, 2024
17a4f38
Update flepimop/gempyor_pkg/src/gempyor/seir.py
emprzy Nov 13, 2024
3acad12
Update flepimop/gempyor_pkg/src/gempyor/seir.py
emprzy Nov 13, 2024
44ccc84
Implementing Tim's suggestions!
emprzy Nov 13, 2024
b32c64e
Regex adjustment
emprzy Nov 13, 2024
d844ced
Regex update
emprzy Nov 13, 2024
2cbf2ed
Implementing Tim's suggestions
emprzy Nov 13, 2024
a1692a1
Regex update in test_statistic_class.py
emprzy Nov 18, 2024
759795f
Regex update in `test_statistic_class.py`
emprzy Nov 18, 2024
df784d5
Regex adjusment in `test_random_distribution_sampler.py`
emprzy Nov 18, 2024
8ec6c05
Fixing typo in `utils.py`
emprzy Nov 18, 2024
d760ddc
Regex update in `test_random_distribution_sampler.py`
emprzy Nov 18, 2024
7e19700
Regex update in `test_random_distribution_sampler.py`
emprzy Nov 18, 2024
452ddbb
Update utils.py
emprzy Nov 20, 2024
99d2f36
Regex update in `test_read_df.py`
emprzy Nov 20, 2024
47514d5
Regex update in `test_read_df.py`
emprzy Nov 20, 2024
97d3996
Regex update in `test_read_df.py`
emprzy Nov 20, 2024
918ebc7
Updating a unit test in `test_utils.py`
emprzy Nov 20, 2024
764c4b2
Regex update in `test_utils.py` and error type agreement with `utils.py`
emprzy Nov 20, 2024
c1e79c2
Regex update in `test_write_df.py`
emprzy Nov 20, 2024
4fd1b11
Regex update in `test-write_df.py`
emprzy Nov 20, 2024
2e62ae3
Removing deprecated style of `\` to escape colons
emprzy Nov 20, 2024
e5772c7
linting with `black`
emprzy Nov 20, 2024
8fe918a
Updating `steps_source.py` and `utils.py`
emprzy Nov 20, 2024
f7e53a7
Implementing Carl's suggestions
emprzy Nov 22, 2024
d5fe360
Regex update in `test_model_info.py`
emprzy Nov 22, 2024
9c6e1f1
Regex update in `test_model_info.py`
emprzy Nov 26, 2024
f7c1ad0
Regex update in `test_model_info.py`
emprzy Nov 26, 2024
ebe2ebc
Typo in `model_info.py`
emprzy Nov 27, 2024
57240be
Using `re.escape()` to escape non-alphanumeric characters in the regex
emprzy Nov 27, 2024
cc52fb7
Regex update in `test_seir.py`
emprzy Dec 2, 2024
328dacf
Syntax update in `test_seir.py`
emprzy Dec 2, 2024
e7621d9
Update test_seir.py
emprzy Dec 2, 2024
d74a214
Regex update in `test_statistic_class.py`
emprzy Dec 2, 2024
d508793
Regex update in `test_statistic_class.py`
emprzy Dec 2, 2024
6fc2a5a
Regex update in `test_statistic_class.py`
emprzy Dec 2, 2024
f8b9dc3
Syntax change in `test_statistic_class.py`
emprzy Dec 2, 2024
ad623ab
Regex update in `test_statistic_class.py`
emprzy Dec 2, 2024
a3b5f62
Testing a regex in `test_statistic_class.py`
emprzy Dec 2, 2024
1daed72
Using re.escape() to escape non-alphanumeric characters in the regex
emprzy Dec 2, 2024
239f5a0
Regex update in `test_random_distribution_sampler.py`
emprzy Dec 2, 2024
50b91ce
Regex update in `test_random_distribution_sampler.py`
emprzy Dec 2, 2024
84fb067
Update test_random_distribution_sampler.py
emprzy Dec 2, 2024
4bf61fb
Update test_random_distribution_sampler.py
emprzy Dec 2, 2024
660795f
Using `re.escape()` in regexs
emprzy Dec 2, 2024
00120cf
Regex update in `test_read_df.py`
emprzy Dec 2, 2024
0a05cb5
Update utils.py
emprzy Dec 2, 2024
190c483
Regex update in `test_utils.py`
emprzy Dec 3, 2024
fb6c9ed
Regex update in `test_utils.py`
emprzy Dec 3, 2024
8255da2
Update test_utils.py
emprzy Dec 3, 2024
8a5e772
Regex update in `test_write_df.py`
emprzy Dec 3, 2024
6ce1760
Update utils.py
emprzy Dec 3, 2024
178861a
Update test_utils.py
emprzy Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flepimop/gempyor_pkg/src/gempyor/inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ def get_all_sim_arguments(self):
def get_logloss(self, proposal):
if not self.inferpar.check_in_bound(proposal=proposal):
if not self.silent:
print("`llik` is -inf (out of bound proposal)")
print("`llik` is -inf (out of bound proposal).")
return -np.inf, -np.inf, -np.inf

snpi_df_mod, hnpi_df_mod = self.inferpar.inject_proposal(
Expand Down
30 changes: 14 additions & 16 deletions flepimop/gempyor_pkg/src/gempyor/model_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __init__(self, config: confuse.ConfigView):
self.tf = config["end_date"].as_date()
if self.tf <= self.ti:
raise ValueError(
"tf (time to finish) is less than or equal to ti (time to start)"
f"Final time ('{self.tf}') is less than or equal to initial time ('{self.ti}')."
)
self.n_days = (self.tf - self.ti).days + 1
self.dates = pd.date_range(start=self.ti, end=self.tf, freq="D")
Expand Down Expand Up @@ -76,9 +76,7 @@ def __init__(
# Auto-detect old config
if config["interventions"].exists():
raise ValueError(
"""This config has an intervention section, and has been written for a previous version of flepiMoP/COVIDScenarioPipeline \
Please use flepiMoP Version 1.1 (Commit SHA: 0c30c23937dd496d33c2b9fa7c6edb198ad80dac) to run this config. \
(use git checkout v1.1 inside the flepiMoP directory)"""
"This config has an intervention section, which is only compatible with a previous version (v1.1) of flepiMoP. "
)

# 1. Create a setup name that contains every scenario.
Expand All @@ -104,7 +102,7 @@ def __init__(
subpop_config = config["subpop_setup"]
if "data_path" in config:
raise ValueError(
"The config has a data_path section. This is no longer supported."
"The config has a `data_path` section. This is no longer supported."
)
self.path_prefix = pathlib.Path(path_prefix)

Expand Down Expand Up @@ -169,19 +167,19 @@ def __init__(
self.seir_modifiers_library = config["seir_modifiers"][
"modifiers"
].get()
raise ValueError(
"Not implemented yet"
raise NotImplementedError(
"This feature has not been implemented yet."
) # TODO create a Stacked from all
elif self.seir_modifiers_scenario is not None:
raise ValueError(
"An seir modifiers scenario was provided to ModelInfo but no 'seir_modifiers' sections in config"
"A `seir_modifiers_scenario` argument was provided to `ModelInfo` but there is no `seir_modifiers` section in the config."
)
else:
logging.info("Running ModelInfo with seir but without SEIR Modifiers")
logging.info("Running `ModelInfo` with seir but without SEIR Modifiers")

elif self.seir_modifiers_scenario is not None:
raise ValueError(
"A seir modifiers scenario was provided to ModelInfo but no 'seir:' sections in config"
"A `seir_modifiers_scenario` argument was provided to `ModelInfo` but there is no `seir` section in the config."
)
else:
logging.critical("Running ModelInfo without SEIR")
Expand All @@ -202,28 +200,28 @@ def __init__(
self.outcome_modifiers_library = config["outcome_modifiers"][
"modifiers"
].get()
raise ValueError(
"Not implemented yet"
raise NotImplementedError(
"This feature has not been implemented yet."
) # TODO create a Stacked from all

## NEED TO IMPLEMENT THIS -- CURRENTLY CANNOT USE outcome modifiers
elif self.outcome_modifiers_scenario is not None:
if config["outcome_modifiers"].exists():
raise ValueError(
"An outcome modifiers scenario was provided to ModelInfo but no 'outcome_modifiers' sections in config"
"A `outcome_modifiers_scenario` argument was provided to `ModelInfo` but there is no `outcome_modifiers` section in the config."
)
else:
self.outcome_modifiers_scenario = None
else:
logging.info(
"Running ModelInfo with outcomes but without Outcomes Modifiers"
"Running `ModelInfo` with outcomes but without Outcomes Modifiers"
)
elif self.outcome_modifiers_scenario is not None:
raise ValueError(
"An outcome modifiers scenario was provided to ModelInfo but no 'outcomes:' sections in config"
"A `outcome_modifiers_scenario` argument was provided to `ModelInfo` but there is no `outcomes` section in the config."
)
else:
logging.info("Running ModelInfo without Outcomes")
logging.info("Running `ModelInfo` without outcomes.")

# 6. Inputs and outputs
if in_run_id is None:
Expand Down
21 changes: 11 additions & 10 deletions flepimop/gempyor_pkg/src/gempyor/outcomes.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def read_parameters_from_config(modinf: model_info.ModelInfo):
branching_data = pa.parquet.read_table(branching_file).to_pandas()
if "relative_probability" not in list(branching_data["quantity"]):
raise ValueError(
f"No 'relative_probability' quantity in {branching_file}, therefor making it useless"
f"There is no `relative_probability` quantity in '{branching_file}'."
)

print(
Expand All @@ -168,7 +168,7 @@ def read_parameters_from_config(modinf: model_info.ModelInfo):
modinf.subpop_struct.subpop_names
):
raise ValueError(
f"Places in seir input files does not correspond to subpops in outcome probability file {branching_file}"
f"SEIR input files do not have subpops that match those in outcome probability file '{branching_file}'."
)

parameters = {}
Expand All @@ -184,7 +184,8 @@ def read_parameters_from_config(modinf: model_info.ModelInfo):

else:
raise ValueError(
f"unsure how to read outcome {new_comp}: not a str, nor an incidence or prevalence: {src_name}"
f"Expected a `str` or `dict` containing `incidence` or `prevalence`. "
f"Instead given '{src_name}' for outcome '{new_comp}'."
)

parameters[new_comp]["probability"] = outcomes_config[new_comp][
Expand Down Expand Up @@ -295,7 +296,7 @@ def read_parameters_from_config(modinf: model_info.ModelInfo):
parameters[new_comp] = {}
parameters[new_comp]["sum"] = outcomes_config[new_comp]["sum"].get()
else:
raise ValueError(f"No 'source' or 'sum' specified for comp {new_comp}")
raise ValueError(f"No `source` or `sum` specified for comp '{new_comp}'.")

return parameters

Expand Down Expand Up @@ -396,15 +397,15 @@ def compute_all_multioutcomes(
)
else:
raise ValueError(
f"Unknown type for seir simulation provided, got f{type(seir_sim)}"
f"Unknown type provided for seir simulation, received '{type(seir_sim)}'."
)
# we don't keep source in this cases
else: # already defined outcomes
if source_name in all_data:
source_array = all_data[source_name]
else:
raise ValueError(
f"ERROR with outcome {new_comp}: the specified source {source_name} is not a dictionnary (for seir outcome) nor an existing pre-identified outcomes."
f"Issue with outcome '{new_comp}'; the specified source '{source_name}' is neither a dictionnary (for seir outcome) nor an existing pre-identified outcome."
)

if (loaded_values is not None) and (
Expand Down Expand Up @@ -590,7 +591,7 @@ def filter_seir_df(diffI, dates, subpops, filters, outcome_name) -> np.ndarray:
vtype = "prevalence"
else:
raise ValueError(
f"Cannot distinguish the source of outcome {outcome_name}: it is not another previously defined outcome and there is no 'incidence:' or 'prevalence:'."
f"Cannot discern the source of outcome '{outcome_name}'; it is not a previously defined outcome and there is no `incidence` or `prevalence`."
)

diffI = diffI[diffI["mc_value_type"] == vtype]
Expand Down Expand Up @@ -619,7 +620,7 @@ def filter_seir_xr(diffI, dates, subpops, filters, outcome_name) -> np.ndarray:
vtype = "prevalence"
else:
raise ValueError(
f"Cannot distinguish the source of outcome {outcome_name}: it is not another previously defined outcome and there is no 'incidence:' or 'prevalence:'."
f"Cannot discern the source of outcome '{outcome_name}'; it is not a previously defined outcome and there is no `incidence` or `prevalence`."
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mild code smell that this exactly duplicates earlier lines 588 ish - extract as helper function, maybe?

# Filter the data
filters = filters[vtype]
Expand Down Expand Up @@ -669,7 +670,7 @@ def multishiftee(arr, shifts, stoch_delay_flag=True):
result = np.zeros_like(arr)

if stoch_delay_flag:
raise ValueError("NOT SUPPORTED YET")
raise NotImplementedError("`stoch_delay_flag` not supported yet.")
# for i, row in reversed(enumerate(np.rows(arr))):
# for j,elem in reversed(enumerate(row)):
## This function takes in :
Expand All @@ -695,7 +696,7 @@ def multishift(arr, shifts, stoch_delay_flag=True):
result = np.zeros_like(arr)

if stoch_delay_flag:
raise ValueError("NOT SUPPORTED YET")
raise NotImplementedError("`stoch_delay_flag` not supported yet.")
# for i, row in reversed(enumerate(np.rows(arr))):
# for j,elem in reversed(enumerate(row)):
## This function takes in :
Expand Down
10 changes: 3 additions & 7 deletions flepimop/gempyor_pkg/src/gempyor/seeding.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@

def _DataFrame2NumbaDict(df, amounts, modinf) -> nb.typed.Dict:
if not df["date"].is_monotonic_increasing:
raise ValueError(
"_DataFrame2NumbaDict got an unsorted dataframe, exposing itself to non-sense"
)
raise ValueError("The `df` given is not sorted by the 'date' column.")

cmp_grp_names = [
col for col in modinf.compartments.compartments.columns if col != "name"
Expand Down Expand Up @@ -111,7 +109,7 @@ def get_from_config(self, sim_id: int, modinf) -> nb.typed.Dict:
dupes = seeding[seeding.duplicated(["subpop", "date"])].index + 1
if not dupes.empty:
raise ValueError(
f"Repeated subpop-date in rows {dupes.tolist()} of seeding::lambda_file."
f"There are repeating subpop-date in rows '{dupes.tolist()}' of `seeding::lambda_file`."
)
elif method == "FolderDraw":
seeding = pd.read_csv(
Expand All @@ -136,7 +134,7 @@ def get_from_config(self, sim_id: int, modinf) -> nb.typed.Dict:
seeding = pd.DataFrame(columns=["date", "subpop"])
return _DataFrame2NumbaDict(df=seeding, amounts=[], modinf=modinf)
else:
raise NotImplementedError(f"unknown seeding method [got: {method}]")
raise ValueError(f"Unknown seeding method given, '{method}'.")

# Sorting by date is very important here for the seeding format necessary !!!!
# print(seeding.shape)
Expand All @@ -160,8 +158,6 @@ def get_from_config(self, sim_id: int, modinf) -> nb.typed.Dict:
)
elif method == "FolderDraw" or method == "FromFile":
amounts = seeding["amount"]
else:
raise ValueError(f"Unknown seeding method: {method}")

return _DataFrame2NumbaDict(df=seeding, amounts=amounts, modinf=modinf)

Expand Down
12 changes: 6 additions & 6 deletions flepimop/gempyor_pkg/src/gempyor/seir.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ def build_step_source_arg(
if integration_method == "rk4":
integration_method = "rk4.jit"
if integration_method not in ["rk4.jit", "legacy"]:
raise ValueError(f"Unknown integration method {integration_method}.")
raise ValueError(
f"Unknown integration method given, '{integration_method}'."
)
if "dt" in modinf.seir_config["integration"].keys():
dt = float(
eval(str(modinf.seir_config["integration"]["dt"].get()))
Expand Down Expand Up @@ -148,8 +150,7 @@ def steps_SEIR(
elif integration_method == "rk4.jit":
if modinf.stoch_traj_flag == True:
raise ValueError(
f"with method {integration_method}, only deterministic "
f"integration is possible (got stoch_straj_flag={modinf.stoch_traj_flag}"
f"'{integration_method}' integration method only supports deterministic integration, but `stoch_straj_flag` is '{modinf.stoch_traj_flag}'."
)
seir_sim = steps_rk4.rk4_integration(**fnct_args, silent=True)
else:
Expand All @@ -164,8 +165,7 @@ def steps_SEIR(
]:
if modinf.stoch_traj_flag == True:
raise ValueError(
f"with method {integration_method}, only deterministic "
f"integration is possible (got stoch_straj_flag={modinf.stoch_traj_flag}"
f"'{integration_method}' integration method only supports deterministic integration, but `stoch_straj_flag` is '{modinf.stoch_traj_flag}'."
)
seir_sim = steps_experimental.ode_integration(
**fnct_args, integration_method=integration_method
Expand All @@ -187,7 +187,7 @@ def steps_SEIR(
elif integration_method == "rk4_aot":
seir_sim = steps_experimental.rk4_aot(**fnct_args)
else:
raise ValueError(f"Unknow integration scheme, got {integration_method}")
raise ValueError(f"Unknown integration method given, '{integration_method}'.")

# We return an xarray instead of a ndarray now
compartment_coords = {}
Expand Down
18 changes: 11 additions & 7 deletions flepimop/gempyor_pkg/src/gempyor/statistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ def __init__(self, name: str, statistic_config: confuse.ConfigView) -> None:
reg_name = reg_config["name"].get()
reg_func = getattr(self, f"_{reg_name}_regularize", None)
if reg_func is None:
raise ValueError(f"Unsupported regularization: {reg_name}")
raise ValueError(
f"Unsupported regularization [received: '{reg_name}']. "
f"Currently only `forecast` and `allsubpop` are supported."
)
self.regularizations.append((reg_func, reg_config.get()))

self.resample = False
Expand Down Expand Up @@ -253,7 +256,10 @@ def llik(self, model_data: xr.DataArray, gt_data: xr.DataArray) -> xr.DataArray:
"absolute_error": lambda x, y: -np.log(np.nansum(np.abs(x - y))),
}
if self.dist not in dist_map:
raise ValueError(f"Invalid distribution specified: {self.dist}")
raise ValueError(
f"Invalid distribution specified: '{self.dist}'. "
f"Valid distributions: '{dist_map.keys()}'."
)
if self.dist in ["pois", "nbinom"]:
model_data = model_data.astype(int)
gt_data = gt_data.astype(int)
Expand Down Expand Up @@ -295,11 +301,9 @@ def compute_logloss(

if not model_data.shape == gt_data.shape:
raise ValueError(
(
f"{self.name} Statistic error: data and groundtruth do not have "
f"the same shape: model_data.shape={model_data.shape} != "
f"gt_data.shape={gt_data.shape}"
)
f"`model_data` and `gt_data` do not have "
f"the same shape: `model_data.shape` = '{model_data.shape}' != "
f"`gt_data.shape` = '{gt_data.shape}'."
)

regularization = 0.0
Expand Down
7 changes: 5 additions & 2 deletions flepimop/gempyor_pkg/src/gempyor/steps_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,11 @@ def steps_SEIR_nb(
print(" ", states_current[comp].max())

if (states_current.min() < 0) or (states_current.max() > 10**10):
print((states_current.min() < 0), (states_current.max() > 10**10))
raise ValueError(f"Overflow error. Too small ?. Too large ?")
raise ValueError(
f"State values are outside the valid range. "
f"Minimum value: '{states_current.min()}', Maximum value: '{states_current.max()}'. "
f"Valid range is between 0 and {10**10}."
)
Comment on lines +281 to +285
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError(
f"State values are outside the valid range. "
f"Minimum value: '{states_current.min()}', Maximum value: '{states_current.max()}'. "
f"Valid range is between 0 and {10**10}."
)
raise ValueError(
f"State min and/or max are outside the valid range. "
f"Minimum value: '{states_current.min()}' vs 0, Maximum value: '{states_current.max()}' vs {10**10}. "
)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I push back on this a little? I feel like the adjustment I made is easy to read (but maybe that's just my brain). Do you feel strongly about this adjustment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not feel strongly.


return states, states_daily_incid

Expand Down
Loading
Loading