Skip to content

Add matched-null validation for cluster counts - #938

Draft
haomeng797-ship-it wants to merge 4 commits into
easystats:mainfrom
haomeng797-ship-it:codex/test-clusters
Draft

Add matched-null validation for cluster counts#938
haomeng797-ship-it wants to merge 4 commits into
easystats:mainfrom
haomeng797-ship-it:codex/test-clusters

Conversation

@haomeng797-ship-it

Copy link
Copy Markdown
Member

Closes easystats/easystats#479.

This adds test_clusters(), which compares a selected cluster count with counts from matched-null data generated by matchednull. The default pipeline standardizes the data and uses mclust::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 ... to matchednull::matched_null_test().

I added tests for custom functions, reproducibility, null and positive controls, argument forwarding, and input validation.

  • 22 targeted tests passed
  • lintr passed
  • package build passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread R/test_clusters.R
Comment on lines +86 to +88
insight::check_if_installed("mclust")
mclustBIC <- mclust::mclustBIC
max_components <- min(as.integer(n_max), nrow(x) - 1L)
Comment thread DESCRIPTION
loo,
MASS,
Matrix,
matchednull (>= 0.2.1),
@strengejacke

Copy link
Copy Markdown
Member

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?

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

Thanks! I bumped the development version to 0.17.1.11.

@DominiqueMakowski

DominiqueMakowski commented Aug 25, 2026

Copy link
Copy Markdown
Member

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 n_clusters argument that takes either a number, or a list of numbers to test multiple solutions.

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

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

Thanks for pointing that out. I agree that the pick_two() example is awkward. The function argument is still needed, though, because the same cluster-selection procedure has to run on every matched-null dataset. Supplying fixed values through n_clusters would not specify how a cluster count should be obtained from each null dataset.

I’ll replace the example with a real clustering pipeline and clarify this in the documentation.

@DominiqueMakowski

Copy link
Copy Markdown
Member

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

n_clusters(data)
Rez <- cluster_analysis(...l

Test_clusters(Rez)
``|

It would actually test against a specific solution 

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

Yes that’s what I mean. test_clusters() needs some way to rerun the clustering method on each matched-null dataset, rather than only knowing the number selected for the original data.

I like the idea of making it a method for a cluster_analysis() result. That would probably feel more natural here. We’d just need the result object to retain the method and arguments, so the same analysis can be rerun on the null datasets.

The only thing I want to preserve is that the current test evaluates the selection process, rather than just one fixed solution.
I’ll look at what cluster_analysis() stores and see how the two could fit together!

@DominiqueMakowski

Copy link
Copy Markdown
Member

I’ll look at what cluster_analysis() stores

if need be we can add more attributes to store more info :) we do that quite commonly across the easyverse

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

I had a look. cluster_analysis() already stores the method, processed data, model, clusters, and performance, so adding the original arguments should be straightforward.

I think the only catch is a fixed n. If rez was fitted with n = 3, running the same thing on every matched-null dataset would just give 3 each time. For the current test, whatever chooses n needs to run again on each dataset so that there’s actually a null distribution.

So test_clusters(rez) could work if rez also remembers how n was chosen. If you mean testing one fixed solution, we’d probably need to compare something like separation or fit instead. Is that roughly what you were thinking?

@DominiqueMakowski

Copy link
Copy Markdown
Member

do you mean that test_clusters need some alternative scenarios (ie different n solutions)?

If so, would something like:

test_clusters(rez, against = c(1, 2, 5)) work?


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 parameters::n_clusters_elbow() returns 3, the idea is to see how clear this selection method's result is on the data vs surrogate clusterless data?

If so... then perhaps test_clusters() should be used within n_clusters()? or as a method for specific cluster number selection methods? I'm confusing myself

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

Yes, your second reading is right! 😀 I think my earlier wording mixed up two possible uses of matched_null_test().

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 n, and against = c(1, 2, 5) is not what I meant.

I think the cleanest API would be something like test_clusters(data, method = "elbow"). It could use the same selection methods as n_clusters(), but remain separate because the simulations can take a while.

Testing a fixed k with something like silhouette width would be another possible test, but that is not what this PR currently does!
Does that sound like the right direction?

@haomeng797-ship-it
haomeng797-ship-it marked this pull request as draft August 27, 2026 14:29
@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

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 🙂

@haomeng797-ship-it

Copy link
Copy Markdown
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: matched-null validation for the easystats clustering workflow

4 participants