[FEAT] enhancement idea for flm #887

Activity

1. tyner

   added

   enhancementNew feature or requestNew feature or request

   on Oct 23, 2026
2. SebKrantz commented on Oct 31, 2026

   

   SebKrantz

   on Oct 31, 2026

   Member

   More actions

   @claude what would be the computational cost of this check for normal use-cases. flm() is designed to be a bootstrap workhorse and should remain as lean as possible. Is there a creative way to add this at extremely low cost?
3. claude commented on Oct 31, 2026

   

   claudeBot

   on Oct 31, 2026 – with Claude

   Last edited by claude

   More actions

   Claude finished @SebKrantz's task in 23s —— View job

   ---

   Cost of the sparse-matrix check in flm()

   I read R/flm.R (flm.default, lines 17-81). I haven't changed any code or run benchmarks, so the timings below are estimates.

   The check can be nearly free if it is written as a short-circuit on a primitive.

   if(isS4(X) && !any(method[1L] == c("qr", "chol"))) # or is.object(X)
     stop("X is a sparse/S4 matrix: only method = 'qr' or 'chol' are supported")

   • isS4() and is.object() are internal primitives that read one bit of the object header. They take roughly 20-50 ns and don't depend on the size of X. They also don't allocate or copy anything.
   • For an ordinary base matrix, isS4(X) is FALSE, so the && stops there. The method comparison never runs, so there is no match.arg, no string comparison and no %in%.
   • A single flm() call already costs tens of microseconds. That covers the switch, attributes<-, .lm.fit or crossprod, and the dim(X)[1L] lookups. The check adds well under 1% of the time of the smallest bootstrap call, so it is negligible even over 10⁶ resamples.
   • The check never touches the data. Its cost stays constant as X grows, whereas the regression itself scales with n·p².

   Other ways to do it, and why I'd skip them

   • tryCatch or withCallingHandlers around the fit. Handler setup costs around 1-3 µs, which is orders of magnitude more than isS4().
   • Putting the check in the error path only, for example by catching the failure from .lm.fit. This adds no cost on success. It needs a handler to be installed up front, though, so it isn't free either. It would also depend on parsing base R's error text, which is fragile.
   • Checking inherits(X, "sparseMatrix"). This is slower (class() plus a string match) and only covers the Matrix package. isS4() also catches other S4 matrix classes, such as dgeMatrix. Those also fail with a cryptic error in .lm.fit.
   • Checking is.matrix(X). Sparse Matrix objects return FALSE, so this would work. It would also reject a bare data.frame or vector, which flm.default currently handles in some paths. That changes behaviour more than necessary.

   Where I'd put it

   • Place it after n <- dim(X)[1L] and before the weights branch, so it applies to both the weighted and unweighted paths and to return.raw = TRUE.
   • flm.formula builds X with do.call(cbind, X) from atomic vectors, so it always produces a base matrix and never hits the check.
   • X * wts with a sparse X goes through Matrix's Arith methods and works only by accident. The isS4 guard also covers the weighted qr and chol paths, which should still work.

   Open question for you: should the error cover only sparseMatrix, or any S4 X when the method isn't qr or chol? I'd go with isS4, as above.

   I haven't implemented this. If you'd like me to, say so and I'll add the guard, a test in tests/testthat/test-flm.R (skipped if Matrix isn't installed), and a NEWS entry. I'd also run a quick microbenchmark on a small X to confirm the overhead is lost in the noise.
   · branch claude/issue-887-20261031-1957
4. SebKrantz commented on Oct 31, 2026

   

   SebKrantz

   on Oct 31, 2026

   Member

   More actions

   @claude thanks. Open a PR to development for the isS4 fix.
5. claude commented on Oct 31, 2026

   

   claudeBot

   on Oct 31, 2026 – with Claude

   Last edited by claude

   More actions

   Claude finished @SebKrantz's task in 44s —— View job • claude/issue-887-20261031-2006 • Create PR ➔

   ---

   flm sparse/S4 guard

   • Add isS4(X) guard to flm.default (R/flm.R). It errors unless method is "qr" or "chol".
   • Add a test to tests/testthat/test-flm-fFtest.R. It is skipped if Matrix isn't installed.
   • Add a NEWS entry.
   • Commit and push.

   The guard sits right after the NROW(y) check, so it covers the weighted, unweighted and return.raw paths. For ordinary matrices isS4(X) is FALSE, so the && stops there and the method comparison never runs.

   I didn't run R: the sandbox blocked the Rscript call. The new test and the guard are untested, so please let CI confirm. I also didn't benchmark the overhead.

   I pushed the branch but didn't open the PR. The link below targets development, as you asked.

   · branch claude/issue-887-20261031-2006