Skip to content

css() aggregation is not order-preserving: a cssLasso() removal can widen selected_feats silently #203

Closed

Description

@gregfaletto

css()'s aggregation is not order-preserving, so a removal at cssLasso() can widen what
cssSelect() and cssPredict() return — through two tie rules, only one of which warns.

Filed out of #199 / #201, which established the behaviour but did not scope characterising it.

The observation

At the cssLasso() level, #201's penalty snap only ever removes a feature: the interpolated
support is a superset of the solved column's, so dropping the interpolation can only shrink the
selected set. That property does not survive aggregation. Per-subsample selections feed cluster
selection proportions, and two tie rules above cssLasso() resolve a tie by returning more:

  1. getSelectedClusters() under max_num_clusts. A removal lowers a cluster's proportion into
    a tie; the cutoff loop cannot raise the cutoff further without returning zero clusters, so it
    returns more than max_num_clusts and warns.
  2. The within-cluster prototype weighting. A tie there widens selected_feats — which is what
    cssPredict() consumes — with selected_clusts unchanged and no warning at all.

Route 2 is the one worth attention: there is no signal a caller can catch.

Measured

Comparing f9a1bb3 (pre-#201) against 3d4f2f4 (the merged branch tip), 400 seeds,
n = 22, p = 14, three named clusters, weak signal:

cluster sets changed : 5   of which pure additions: 3     (seeds 82, 252, 263)
feature sets changed : 8   of which pure additions: 5
  feature additions with NO max_num_clusts warning: 2     (seeds 336, 390)
max_num_clusts warning fired : base 9, head 11

Under options(warn = 2), seeds 82/252/263 error on the new code where the old returned a
result. That consequence is pre-existing in kind — the warning could always fire — but the change
makes it reachable on inputs where it previously was not.

set.seed(336); n <- 22; p <- 14
X <- matrix(rnorm(n * p), n, p)
y <- as.numeric(X %*% c(rep(0.5, 2), rep(0, p - 2)) + rnorm(n))
cssSelect(X = X, y = y, clusters = list(c1 = 1:3, c2 = 4:6, c3 = 7:9))
# pre-#201 : selected_feats = 1        selected_clusts = c1
# post     : selected_feats = 1, 2     selected_clusts = c1     -- no warning

The rate is a property of this configuration (small n, weak signal, default lambda = NA), not
a general one. On a separate 400-seed run at n = 80, p = 40 with signal, no subsample reached
the regime at all.

What this issue is for

Not the widening itself — that is the documented tie rules behaving as specified, and NEWS.md
discloses both routes. The open questions:

  • Is the tie surface characterised anywhere? Both routes above were found by accident, each
    while checking something unrelated. Neither is covered by a test that would notice if a future
    change altered it, and the second is not mentioned in any function's documentation.
  • Should route 2 be silent? A widened selected_feats with an unchanged selected_clusts and
    no condition raised is invisible to a caller who is watching for warnings. Options include
    warning on it as route 1 does, documenting it as expected, or leaving it — but the choice has
    not been made deliberately.
  • Do the tie rules deserve direct tests? getSelectedClusters()'s cutoff loop and the
    prototype weighting each have a documented tie behaviour that nothing pins. The suite reaches
    both only incidentally, through blocks aimed at other things.

Not urgent, and not a defect

Both rules do what they are specified to do, and #201 shipped with both disclosed. This is about
whether the behaviour is characterised and pinned rather than whether it is correct. It is filed
because the two routes were each discovered by accident, which suggests there may be more of the
surface than three sweeps have mapped.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions