From 9c05871fcf5e6f9c4a8f3cea089d284d89f4a78c Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Tue, 20 Oct 2026 22:17:57 -0300 Subject: [PATCH 1/6] fix(metrics): ignore NaN target/prediction pairs in validation metrics (SIENTIAPDE-2215) --- .../repository/data_manager_repository.py | 17 +++++++++ .../test_data_manager_repository.py | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/model_manager/utils/repository/data_manager_repository.py b/model_manager/utils/repository/data_manager_repository.py index 03b31ec..856976f 100644 --- a/model_manager/utils/repository/data_manager_repository.py +++ b/model_manager/utils/repository/data_manager_repository.py @@ -405,6 +405,23 @@ class DataManagerRepository(SientiaMonitoring): y_true_val = y_true_val.loc[common_index] y_pred_val = y_pred_val.loc[common_index] + # NaN target, or NaN prediction for rows with NaN features, carries no error signal + # (SIENTIAPDE-2215). + valid = y_true_val.notna() & y_pred_val.notna() + if not valid.any(): + raise ValueError( + 'No validation rows with both target and prediction: all ' + f'{len(valid)} overlapping rows have NaN' + ) + if not valid.all(): + self.info( + f'Ignoring {int((~valid).sum())} validation row(s) with NaN target or prediction ' + 'in the regression metrics', + metadata, + ) + y_true_val = y_true_val[valid] + y_pred_val = y_pred_val[valid] + # Metrics helpers already round to 2 decimals. tmr.mse_val = mse(y_true_val, y_pred_val) tmr.mae_val = mae(y_true_val, y_pred_val) diff --git a/tests/utils/repository/test_data_manager_repository.py b/tests/utils/repository/test_data_manager_repository.py index 09c52b3..c198efe 100644 --- a/tests/utils/repository/test_data_manager_repository.py +++ b/tests/utils/repository/test_data_manager_repository.py @@ -309,6 +309,41 @@ def test_compute_regression_metrics_no_overlap(): repo.compute_regression_metrics(tmr, MagicMock()) +def test_compute_regression_metrics_ignores_nan_pairs(): + """NaN target or NaN prediction rows are left out of mse/mae/r2 (SIENTIAPDE-2215).""" + repo = dmr.DataManagerRepository(MagicMock()) + p = TrainModelParams.from_dict(_minimal_dict_for_prepare()) + idx = pd.Index([0, 1, 2, 3]) + tmr = TrainModelResult( + params=p, + train_data=pd.DataFrame({'t': [1.0, 2.0, 3.0, 4.0]}, index=idx), + val_data=pd.DataFrame({'t': [1.0, np.nan, 3.0, 4.0]}, index=idx), + y_pred=pd.DataFrame({'p': [1.0, 2.0, np.nan, 4.0]}, index=idx), + ) + + wrapper = MagicMock() + wrapper.model = object() # no sklearn .regr -> equation extraction is skipped + out = repo.compute_regression_metrics(tmr, wrapper) + + assert out.mse_val == 0.0 + assert out.mae_val == 0.0 + + +def test_compute_regression_metrics_all_nan_pairs_raises(): + repo = dmr.DataManagerRepository(MagicMock()) + p = TrainModelParams.from_dict(_minimal_dict_for_prepare()) + idx = pd.Index([0, 1]) + tmr = TrainModelResult( + params=p, + train_data=pd.DataFrame({'t': [1.0, 2.0]}, index=idx), + val_data=pd.DataFrame({'t': [1.0, 2.0]}, index=idx), + y_pred=pd.DataFrame({'p': [np.nan, np.nan]}, index=idx), + ) + + with pytest.raises(ValueError, match='No validation rows with both target and prediction'): + repo.compute_regression_metrics(tmr, MagicMock()) + + def test_compute_regression_metrics_linear_equation(): repo = dmr.DataManagerRepository(MagicMock()) p = TrainModelParams.from_dict(_minimal_dict_for_prepare()) From 8c81aa68bb3a12f756a3756941e35948eadd9e75 Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Wed, 21 Oct 2026 09:11:40 -0300 Subject: [PATCH 2/6] refactor(metrics): warn with the remaining count and document NaN pairs (SIENTIAPDE-2215) --- .../utils/repository/data_manager_repository.py | 14 ++++++++++---- .../repository/test_data_manager_repository.py | 6 +++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/model_manager/utils/repository/data_manager_repository.py b/model_manager/utils/repository/data_manager_repository.py index 856976f..179c3da 100644 --- a/model_manager/utils/repository/data_manager_repository.py +++ b/model_manager/utils/repository/data_manager_repository.py @@ -357,7 +357,8 @@ class DataManagerRepository(SientiaMonitoring): This helper mirrors the previous TrainingRepository.after_train_calculation behavior, assuming that predictions (y_pred/y_train_pred) are already on the correct scale for metric calculation (any scaling is handled inside the - model wrapper). + model wrapper). Pairs with a NaN target or a NaN prediction are left out of + mse/mae/r2. Args: tmr: Training result containing: @@ -368,6 +369,10 @@ class DataManagerRepository(SientiaMonitoring): Return: TrainModelResult: Same object with mse_val, mae_val and r2_val set. + + Raises: + ValueError: If y_pred is not set, val_data and y_pred share no indices, + or no pair has both a valid target and a valid prediction. """ if tmr.y_pred is None: raise ValueError('y_pred must be set before computing regression metrics') @@ -414,9 +419,10 @@ class DataManagerRepository(SientiaMonitoring): f'{len(valid)} overlapping rows have NaN' ) if not valid.all(): - self.info( - f'Ignoring {int((~valid).sum())} validation row(s) with NaN target or prediction ' - 'in the regression metrics', + n_valid = int(valid.sum()) + self.warning( + f'Ignoring {len(valid) - n_valid} of {len(valid)} validation row(s) with NaN ' + f'target or prediction; regression metrics use the remaining {n_valid}', metadata, ) y_true_val = y_true_val[valid] diff --git a/tests/utils/repository/test_data_manager_repository.py b/tests/utils/repository/test_data_manager_repository.py index c198efe..41d159a 100644 --- a/tests/utils/repository/test_data_manager_repository.py +++ b/tests/utils/repository/test_data_manager_repository.py @@ -323,10 +323,14 @@ def test_compute_regression_metrics_ignores_nan_pairs(): wrapper = MagicMock() wrapper.model = object() # no sklearn .regr -> equation extraction is skipped - out = repo.compute_regression_metrics(tmr, wrapper) + with patch.object(repo, 'warning') as warn: + out = repo.compute_regression_metrics(tmr, wrapper) assert out.mse_val == 0.0 assert out.mae_val == 0.0 + assert out.r2_val == 1.0 + warn.assert_called_once() + assert 'Ignoring 2 of 4' in warn.call_args.args[0] def test_compute_regression_metrics_all_nan_pairs_raises(): From 5043137f9701c4b2e5b55dd273f0e720b8dea1e8 Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Wed, 21 Oct 2026 09:32:26 -0300 Subject: [PATCH 3/6] refactor(metrics): build the NaN-pairs warning before the call (SIENTIAPDE-2215) --- model_manager/utils/repository/data_manager_repository.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/model_manager/utils/repository/data_manager_repository.py b/model_manager/utils/repository/data_manager_repository.py index 179c3da..33454ca 100644 --- a/model_manager/utils/repository/data_manager_repository.py +++ b/model_manager/utils/repository/data_manager_repository.py @@ -420,11 +420,11 @@ class DataManagerRepository(SientiaMonitoring): ) if not valid.all(): n_valid = int(valid.sum()) - self.warning( + message = ( f'Ignoring {len(valid) - n_valid} of {len(valid)} validation row(s) with NaN ' - f'target or prediction; regression metrics use the remaining {n_valid}', - metadata, + f'target or prediction; regression metrics use the remaining {n_valid}' ) + self.warning(message, metadata) y_true_val = y_true_val[valid] y_pred_val = y_pred_val[valid] From 959a430314e92c4d702fb7e258e2f29cbb025aff Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Wed, 21 Oct 2026 09:43:49 -0300 Subject: [PATCH 4/6] test(metrics): compare metric floats with pytest.approx (SIENTIAPDE-2215) --- tests/utils/repository/test_data_manager_repository.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/utils/repository/test_data_manager_repository.py b/tests/utils/repository/test_data_manager_repository.py index 41d159a..52eddef 100644 --- a/tests/utils/repository/test_data_manager_repository.py +++ b/tests/utils/repository/test_data_manager_repository.py @@ -326,9 +326,9 @@ def test_compute_regression_metrics_ignores_nan_pairs(): with patch.object(repo, 'warning') as warn: out = repo.compute_regression_metrics(tmr, wrapper) - assert out.mse_val == 0.0 - assert out.mae_val == 0.0 - assert out.r2_val == 1.0 + assert out.mse_val == pytest.approx(0.0) + assert out.mae_val == pytest.approx(0.0) + assert out.r2_val == pytest.approx(1.0) warn.assert_called_once() assert 'Ignoring 2 of 4' in warn.call_args.args[0] From fcb9bb263ba352959a5a05361951ad156891a298 Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Thu, 22 Oct 2026 10:34:00 -0300 Subject: [PATCH 5/6] test(metrics): build the metadata mock outside pytest.raises (SIENTIAPDE-2215) --- tests/utils/repository/test_data_manager_repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/utils/repository/test_data_manager_repository.py b/tests/utils/repository/test_data_manager_repository.py index 52eddef..85abfe7 100644 --- a/tests/utils/repository/test_data_manager_repository.py +++ b/tests/utils/repository/test_data_manager_repository.py @@ -343,9 +343,10 @@ def test_compute_regression_metrics_all_nan_pairs_raises(): val_data=pd.DataFrame({'t': [1.0, 2.0]}, index=idx), y_pred=pd.DataFrame({'p': [np.nan, np.nan]}, index=idx), ) + metadata = MagicMock() with pytest.raises(ValueError, match='No validation rows with both target and prediction'): - repo.compute_regression_metrics(tmr, MagicMock()) + repo.compute_regression_metrics(tmr, metadata) def test_compute_regression_metrics_linear_equation(): From 9fa1430d75e9ceaf44ceddd4dc3e257f12b75017 Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Thu, 22 Oct 2026 10:57:53 -0300 Subject: [PATCH 6/6] fix(metrics): name the wrapper mock correctly and document NaN r2 with one valid pair (SIENTIAPDE-2215) --- model_manager/utils/repository/data_manager_repository.py | 2 +- tests/utils/repository/test_data_manager_repository.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/model_manager/utils/repository/data_manager_repository.py b/model_manager/utils/repository/data_manager_repository.py index 33454ca..1ad874b 100644 --- a/model_manager/utils/repository/data_manager_repository.py +++ b/model_manager/utils/repository/data_manager_repository.py @@ -358,7 +358,7 @@ class DataManagerRepository(SientiaMonitoring): behavior, assuming that predictions (y_pred/y_train_pred) are already on the correct scale for metric calculation (any scaling is handled inside the model wrapper). Pairs with a NaN target or a NaN prediction are left out of - mse/mae/r2. + mse/mae/r2. With a single valid pair left, r2_val is NaN (R² needs 2 samples). Args: tmr: Training result containing: diff --git a/tests/utils/repository/test_data_manager_repository.py b/tests/utils/repository/test_data_manager_repository.py index 85abfe7..c0b7e7c 100644 --- a/tests/utils/repository/test_data_manager_repository.py +++ b/tests/utils/repository/test_data_manager_repository.py @@ -343,10 +343,10 @@ def test_compute_regression_metrics_all_nan_pairs_raises(): val_data=pd.DataFrame({'t': [1.0, 2.0]}, index=idx), y_pred=pd.DataFrame({'p': [np.nan, np.nan]}, index=idx), ) - metadata = MagicMock() + wrapper = MagicMock() with pytest.raises(ValueError, match='No validation rows with both target and prediction'): - repo.compute_regression_metrics(tmr, metadata) + repo.compute_regression_metrics(tmr, wrapper) def test_compute_regression_metrics_linear_equation():