diff --git a/src/vak/eval/frame_classification.py b/src/vak/eval/frame_classification.py index 665a0bb4f..f14eae17e 100644 --- a/src/vak/eval/frame_classification.py +++ b/src/vak/eval/frame_classification.py @@ -1,5 +1,4 @@ """Function that evaluates trained models in the frame classification family.""" - from __future__ import annotations import json @@ -30,7 +29,7 @@ def eval_frame_classification_model( num_workers: int, frames_standardizer_path: str | pathlib.Path = None, post_tfm_kwargs: dict | None = None, -) -> None: +) -> dict[str, torch.Tensor]: """Evaluate a trained model. Parameters @@ -215,10 +214,11 @@ def eval_frame_classification_model( logger=trainer_logger, ) # TODO: check for hasattr(model, test_step) and if so run test + # below, [0] because validate returns list of dicts, length of no. of val loaders metric_vals = trainer.validate(model, dataloaders=val_loader)[0] - metric_vals = {f"avg_{k}": v for k, v in metric_vals.items()} - for metric_name, metric_val in metric_vals.items(): + metric_vals_for_df = {f"avg_{k}": v for k, v in metric_vals.items()} + for metric_name, metric_val in metric_vals_for_df.items(): if metric_name.startswith("avg_"): logger.info(f"{metric_name}: {metric_val:0.5f}") @@ -237,7 +237,7 @@ def eval_frame_classification_model( # order metrics by name to be extra sure they will be consistent across runs row.update( sorted( - [(k, v) for k, v in metric_vals.items() if k.startswith("avg_")] + [(k, v) for k, v in metric_vals_for_df.items() if k.startswith("avg_")] ) ) @@ -249,3 +249,6 @@ def eval_frame_classification_model( eval_df.to_csv( eval_csv_path, index=False ) # index is False to avoid having "Unnamed: 0" column when loading + + return metric_vals + diff --git a/src/vak/transforms/frame_labels/functional.py b/src/vak/transforms/frame_labels/functional.py index e3b89bca5..0c05a68dd 100644 --- a/src/vak/transforms/frame_labels/functional.py +++ b/src/vak/transforms/frame_labels/functional.py @@ -556,6 +556,15 @@ def postprocess( frame_labels : numpy.ndarray Vector of frame labels after post-processing is applied. """ + # NOTE: we need to clone to avoid mutating tensors inside a model, + # since calling `torch.tensor.numpy()` inside the model gives us + # a numpy array that shares memory with the tensor. + # Not doing so can make metrics wrong, + # e.g., for frame labels *before* post-processing, + # if we mutate that tensor here. + # see:https://github.com/vocalpy/vak/issues/821 + frame_labels = np.copy(frame_labels) + frame_labels = row_or_1d(frame_labels) if boundary_labels is not None: boundary_labels = row_or_1d(boundary_labels) diff --git a/tests/test_eval/test_frame_classification.py b/tests/test_eval/test_frame_classification.py index 8ac65539a..3c58cade9 100644 --- a/tests/test_eval/test_frame_classification.py +++ b/tests/test_eval/test_frame_classification.py @@ -2,13 +2,11 @@ import pytest import vak.config -import vak.common.constants -import vak.common.paths import vak.eval.frame_classification # written as separate function so we can re-use in tests/unit/test_cli/test_eval.py -def assert_eval_output_matches_expected(model_name, output_dir): +def assert_eval_saves_one_csv(model_name, output_dir): eval_csv = sorted(output_dir.glob(f"eval_{model_name}*csv")) assert len(eval_csv) == 1 @@ -22,9 +20,13 @@ def assert_eval_output_matches_expected(model_name, output_dir): # use ToLabelsWithPostprocessing with *just* majority_vote {'majority_vote': True, 'min_segment_dur': None}, # use ToLabelsWithPostprocessing with *just* min_segment_dur - {'majority_vote': False, 'min_segment_dur': 0.002}, + # NOTE the value of `min_segment_dur=0.05` will make metrics after post-processing *worse* + # but we do that to make sure that this parameter **alone** is having some effect. + # For our current test data, the "good" value of 0.01 we used can in some cases + # have no effect; we want to be sure that's not due to some bug + {'majority_vote': False, 'min_segment_dur': 0.05}, # use ToLabelsWithPostprocessing with majority_vote *and* min_segment_dur - {'majority_vote': True, 'min_segment_dur': 0.002}, + {'majority_vote': True, 'min_segment_dur': 0.05}, ] @@ -69,7 +71,7 @@ def test_eval_frame_classification_model( ) cfg = vak.config.Config.from_toml_path(toml_path) - vak.eval.frame_classification.eval_frame_classification_model( + metric_vals: dict = vak.eval.frame_classification.eval_frame_classification_model( model_config=cfg.eval.model.asdict(), dataset_config=cfg.eval.dataset.asdict(), trainer_config=cfg.eval.trainer.asdict(), @@ -81,7 +83,34 @@ def test_eval_frame_classification_model( post_tfm_kwargs=post_tfm_kwargs, ) - assert_eval_output_matches_expected(cfg.eval.model.name, output_dir) + assert_eval_saves_one_csv(cfg.eval.model.name, output_dir) + + # REGRESSION TEST: these assertions test that + # we fixed https://github.com/vocalpy/vak/issues/821 + # Long-term this may not be the right place to test this, + # but it will work to get out a quick fix; + # NOTE also this is fragile -- I expect the transforms to change the metrics + # for this exact data, but of course you could have a case where + # neither `majority_vote` nor `min_segment_dur` have any effect + # even when `majority_vote=True` and `min_segment_dur > 0.0 + if post_tfm_kwargs is not None: + if post_tfm_kwargs["majority_vote"] is True or ( + post_tfm_kwargs["min_segment_dur"] is not None and post_tfm_kwargs["min_segment_dur"] > 0.0 + ): + for metric_name in ( + "acc", "levenshtein", "character_error_rate", + ): + wout_tfm = metric_vals[f"val_{metric_name}_epoch"] + with_tfm = metric_vals[f"val_{metric_name}_tfm_epoch"] + assert wout_tfm != with_tfm + # for IR metrics, sometimes recall is unchanged + # we mainly want to be sure they are not *all* literally the sma value + assert not all( + [ + metric_vals[f"val_{ir_metric_name}_epoch"] == metric_vals[f"val_{ir_metric_name}_tfm_epoch"] + for ir_metric_name in ("precision", "recall", "fscore", "rval") + ] + ) @pytest.mark.parametrize( diff --git a/tests/test_eval/test_parametric_umap.py b/tests/test_eval/test_parametric_umap.py index b1744291d..d37cafa9e 100644 --- a/tests/test_eval/test_parametric_umap.py +++ b/tests/test_eval/test_parametric_umap.py @@ -7,7 +7,7 @@ import vak.eval.frame_classification -def assert_eval_output_matches_expected(model_name, output_dir): +def assert_eval_saves_one_csv(model_name, output_dir): eval_csv = sorted(output_dir.glob(f"eval_{model_name}*csv")) assert len(eval_csv) == 1 @@ -57,7 +57,7 @@ def test_eval_parametric_umap_model( num_workers=cfg.eval.num_workers, ) - assert_eval_output_matches_expected(cfg.eval.model.name, output_dir) + assert_eval_saves_one_csv(cfg.eval.model.name, output_dir) @pytest.mark.parametrize(