diff --git a/inst/tinytest/test_utils_anomaly_score.R b/inst/tinytest/test_utils_anomaly_score.R index 70959e05..7bc72f29 100644 --- a/inst/tinytest/test_utils_anomaly_score.R +++ b/inst/tinytest/test_utils_anomaly_score.R @@ -383,5 +383,30 @@ duplicate_metrics = run_quality_metrics( # The last 5 rows (with high values) should have lower mean anomaly scores # Since they are all clumped between 2 and 4, whereas 0.1 is by itself expect_true(mean(duplicate_metrics$AnomalyScores[6:10]) < mean(duplicate_metrics$AnomalyScores[1:5]), - info = "Rows 6-10 (values clumped 2-4) should have lower + info = "Rows 6-10 (values clumped 2-4) should have lower anomaly scores than rows 1-5 (isolated value of 0.1)") + +nan_first_row_df = create_base_df(5) +nan_first_row_df$QualityMetric.mean_increase = c(NA, 0.2, 0.4, 0.6, 0.8) + +nan_first_row_result = tryCatch({ + MSstatsConvert:::.runAnomalyModel( + nan_first_row_df, + n_trees = 100, + max_depth = "auto", + cores = 1, + split_column = "PSM", + quality_metrics = c("QualityMetric.mean_increase")) +}, error = function(e) e) + +expect_false(inherits(nan_first_row_result, "error"), + info = paste( + "Anomaly model should not crash/error when a quality metric has a", + "leading NA/NaN value within a PSM group.", + if (inherits(nan_first_row_result, "error")) + paste("Got error:", conditionMessage(nan_first_row_result)) else "")) + +if (!inherits(nan_first_row_result, "error")) { + expect_true(all(is.finite(nan_first_row_result$AnomalyScores)), + info = "Anomaly scores should be finite even when a quality metric has a leading missing value") +} diff --git a/src/isolation_forest.cpp b/src/isolation_forest.cpp index 7ac6dae9..ad82975d 100644 --- a/src/isolation_forest.cpp +++ b/src/isolation_forest.cpp @@ -78,22 +78,30 @@ std::unique_ptr isolation_tree( std::string split_feature = features[feature_dist(gen)]; // Can split on numeric or missing value - double min_val = data[0].at(split_feature); - double max_val = min_val; + double min_val = 0.0; + double max_val = 0.0; bool has_missing = false; + bool has_valid = false; for (const auto& row : data) { - if (std::isnan(row.at(split_feature))) { + double val = row.at(split_feature); + if (std::isnan(val)) { has_missing = true; continue; } - min_val = std::min(min_val, row.at(split_feature)); - max_val = std::max(max_val, row.at(split_feature)); + if (!has_valid) { + min_val = val; + max_val = val; + has_valid = true; + } else { + min_val = std::min(min_val, val); + max_val = std::max(max_val, val); + } } - - if (min_val == max_val && !has_missing) { + + if (!has_valid || (min_val == max_val && !has_missing)) { return std::make_unique(n); } - + // TODO: Chance to chose missing is 50/50. Could make less likely. Test bool is_missing_split = false; double split_value = 0.0;