Add matched-null validation for cluster counts - #938
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new public helper, test_clusters(), to validate an observed (or user-supplied) cluster count against matched-null reference data generated via matchednull, with a default mclust::Mclust() (BIC) pipeline and support for custom scalar-returning cluster functions.
Changes:
- Introduces
test_clusters()(exported) with roxygen documentation and matched-null integration. - Adds a focused test suite covering custom functions, reproducibility, controls, argument forwarding, and input validation.
- Updates package metadata/documentation (NEWS entry, DESCRIPTION Suggests, WORDLIST, Rd generation, NAMESPACE export).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
R/test_clusters.R |
Adds the new test_clusters() API plus internal validation/standardization helpers. |
tests/testthat/test-test_clusters.R |
Adds test coverage for the new function’s expected behavior and input checks. |
NEWS.md |
Documents the new user-facing function in the changelog. |
NAMESPACE |
Exports test_clusters() as part of the public API. |
man/test_clusters.Rd |
Generated documentation for test_clusters(). |
DESCRIPTION |
Adds matchednull (>= 0.2.1) to Suggests for the new functionality/tests. |
inst/WORDLIST |
Adds “Meng” to the spelling whitelist for the new reference entry. |
Files not reviewed (1)
- man/test_clusters.Rd: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| insight::check_if_installed("mclust") | ||
| mclustBIC <- mclust::mclustBIC | ||
| max_components <- min(as.integer(n_max), nrow(x) - 1L) |
| loo, | ||
| MASS, | ||
| Matrix, | ||
| matchednull (>= 0.2.1), |
|
Thanks, looks good to me! A version bump (4th digits, to increase dev version) in the DESCRIPTION is missing (or overwritten by merge main into this branch), else I have no other comments. @DominiqueMakowski anything you would like to add? |
|
Thanks! I bumped the development version to 0.17.1.11. |
|
This is a bit clunky #' # Any scalar-returning clustering pipeline can be tested.
#' pick_two <- function(data) 2
#' test_clusters(iris[, 1:4], cluster_function = pick_two, iterations = 19)Rather than having the whole Mclust complication (which is used to get a number), why not streamline the api and just have the The pipeline could then be like n <- n_clusters(data)
# this suggests 3
test_clusters(data, n_clusters = c(2, 3, 5))or am i missing somethign |
|
Thanks for pointing that out. I agree that the I’ll replace the example with a real clustering pipeline and clarify this in the documentation. |
|
You mean the function needs to be aware of the clustering method? Then why not (I'm just brainstorming I dont have strong opinions) making the function a method that runs on a cluster_analysis() result? So the workflow would go like |
|
Yes that’s what I mean. I like the idea of making it a method for a The only thing I want to preserve is that the current test evaluates the selection process, rather than just one fixed solution. |
if need be we can add more attributes to store more info :) we do that quite commonly across the easyverse |
|
I had a look. I think the only catch is a fixed So |
|
do you mean that test_clusters need some alternative scenarios (ie different n solutions)? If so, would something like:
I reread you message again, and now I am starting to doubt my understanding (sorry i should have done my homework before 😬): do you mean that the matched-null validation is not about testing a given solution, but rather testing a selection procedure? Like if If so... then perhaps |
|
Yes, your second reading is right! 😀 I think my earlier wording mixed up two possible uses of For this PR, the statistic is the number of clusters selected. We run the same selector on the real data and on every matched-null dataset. For example, if elbow selects 3 on the real data but usually selects 1 on the null data, then 3 is unusual. If it also often selects 3 or more on the null data, then the result is not much evidence for real clusters. So there is no need for the user to provide alternative values of I think the cleanest API would be something like Testing a fixed |
|
Quick update: this discussion made me go back and check the null generator before changing the API. I found a dependence-matching issue for non-Gaussian margins in matchednull, so I want to finish validating the correction before this is merged. I'm going to pause this PR for now and come back with the updated calibration. Nothing needed from you in the meantime 🙂 |
|
I opened a separate discussion for the statistical side of this, so we can keep this PR focused on the performance API: easystats/easystats#481 |
Closes easystats/easystats#479.
This adds
test_clusters(), which compares a selected cluster count with counts from matched-null data generated bymatchednull. The default pipeline standardizes the data and usesmclust::Mclust()to select the number of components by BIC; custom scalar-returning clustering functions are also supported.The public API keeps the main controls explicit, while options such as the copula and parallel evaluation can be passed through
...tomatchednull::matched_null_test().I added tests for custom functions, reproducibility, null and positive controls, argument forwarding, and input validation.
lintrpassed