Conversation
Fixed `net_regression()` failing on a two-mode network with more columns than rows (closing #4). The validity mask was built as rows-by-rows, so a wider predictor extended it with NA and the dyad count came back as NA. Renamed `reps` to `times`, `nullhyp`/`method` to `permute` with the values `"predictor"` and `"outcome"`, and `mode` to a logical `directed`. Retired `data` as an identifier: `matlist` is the list of matrices the engine fits and `net` is one coerced network in the formula front end. Added reporting of every default the model resolves for itself, and test-qap_reporting.R, which runs with snet_verbosity = "verbose" so that a message cli cannot parse cannot stay invisible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved to feature/torch-gpu. Reinstate with `git revert` of this commit.
`gpu_batch_ols()` ran only where the family is gaussian and there were no
random effects, no fixed effects, and no multinomial comparison. It had no
test, no hosted runner has a CUDA device to exercise it, and having {torch}
in Suggests broke the CI build: torch installs as an R package before its
Lantern backend, so `cuda_is_available()` throws rather than returning FALSE.
Removes R/qap_gpu.R, the `use_gpu` control, and the gate in both engines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved to feature/gmm-estimator. Reinstate with `git revert` of this commit. The GMM path warned "the covariance matrix of the coefficients is singular" on every family it declares, on well-conditioned data with real signal, so its standard errors are not what they claim to be. It returns coefficients, so a test that only checks the shape passes. Removes R/qap_gmm.R, the 68-line branch in fit_qap_model(), and the `estimator` control, which had one remaining value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved to feature/glmmtmb-mixed. Reinstate with `git revert` of this commit.
Mixed negative binomial and mixed zero-inflated Poisson were the only two
paths needing {glmmTMB}, which carries 62 recursive dependencies, an order
of magnitude more than anything else in Suggests, and pulls {lme4} anyway.
Its build must match {TMB}; the pair already fell out of step locally, so
both paths went untested.
Each combination now aborts and names the alternative. The standard zip
path through {pscl} and the standard negbin path through {MASS} are
unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved to feature/multinomial-comparison. Reinstate with `git revert` of
this commit.
`family = "multinom"` coerced the outcome to a factor, so a numeric tie
weight produced one level per distinct weight and {nnet} refused. No call
through `net_regression()` reached it.
The `comparison` and `reference` controls belonged to it: they ran a
pairwise branch returning a list of p-value matrices instead of one, so the
shape of the result changed. That fork appeared at 21 points across the two
engines, four of them reading
`if (is.null(comparison)) fit$base else fit$base` -- the same value on both
arms. Neither control was documented beyond a line, and neither was tested.
This is the largest obstacle to merging the two engines, so it goes first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved to feature/fixest-fixed-effects. Reinstate with `git revert` of this
commit.
A bar in the formula meant two things. `getRHSNames()` read `y ~ a + b | c`
as an lme4 random slope and rewrote it as `(b | c)`, while
`parse_qap_formula()` read a bar without parentheses as a fixest fixed
effect. Because the front end always adds the parentheses, fixed effects
through the formula were unreachable, and {fixest} was only ever entered
through `fixest_se_cluster`.
With {fixest} on a branch a bar means an {lme4} random-effect term, and
nothing else. `parse_qap_formula()` drops from three branches to one.
Removes the `fixest_se_cluster` control and the fixest branch in
fit_qap_model().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`QAPglm()` and `QAPcss()` were two 200-line functions that were 55% the same code, so every fix had to be made twice, and one of them was made in only one place. They are now `QAPengine()`, which fits both a dyadic network and a cognitive social structure. What the two shapes do differently is four functions in R/qap_shapes.R: how to vectorise a network into rows, how to draw a permutation, how to put a residualised predictor back, and which random intercepts exist. A fifth field says how many permutations to redraw before giving up: one for a dyadic network, 10,000 for a sparse CSS array. A random-intercept slot a shape does not list now aborts by name, so a perceiver intercept on a dyadic network says so rather than building a formula that will not parse. The engine files fall from 791 lines to 552. Also registers `print.QAPCSS()`, which was never an S3 method, and removes a dangling GMM line left in it by an earlier strip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The current changes introduce at least two user-visible issues (over-strict groups validation for rectangular/two-mode outcomes and confusing GLM print-table columns mixing Exp(b) with t-based permutation p-values).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR prepares the infernet R package for v0.2.0 by refactoring and simplifying the QAP regression engine, standardizing vocabulary across the formula front end and engine, and adding regression tests for previously failing two-mode shapes (Issue #4).
Changes:
- Fixes
net_regression()on wide two-mode networks by removing square-matrix assumptions in dyad counting/vectorisation. - Replaces separate
QAPglm()/QAPcss()engines with a unifiedQAPengine()plus “shape” adapters, while removing several parked/optional estimator paths (GPU, GMM, fixest, multinom, mixed glmmTMB). - Adds verbose reporting coverage and new fixtures/tests for wide two-mode and CSS shapes; updates docs/NEWS and reduces
Suggests.
File summaries
| File | Description |
|---|---|
tests/testthat/test-qap_shapes.R |
Updates directedness assertions; adds wide two-mode regression tests for #4 and make_qap_data() shape test. |
tests/testthat/test-qap_shape_css.R |
Adds CSS-focused tests to validate the merged engine’s CSS shape path. |
tests/testthat/test-qap_reporting.R |
Adds tests ensuring verbose snet_info() reporting messages render correctly. |
tests/testthat/test-qap_estimators.R |
Removes GMM, fixest, and GPU path tests to match simplified estimator surface. |
tests/testthat/test-qap_control.R |
Updates control defaults and naming (permute, directed) and related behavioral expectations. |
tests/testthat/test-net_regression.R |
Updates method/nullhyp assertions to permute recording on fits. |
tests/testthat/test-model_tests.R |
Renames result fields (directed, times) to match standardized vocabulary. |
tests/testthat/helper-infernet.R |
Adds a wide two-mode fixture; updates helper signatures to use directed. |
R/qap_utils.R |
Renames parameters (times, directed), updates vectorisation logic for rectangular matrices, and simplifies estimator dispatch. |
R/qap_shapes.R |
Introduces shape abstraction to unify dyadic vs CSS handling in QAPengine(). |
R/qap_misc.R |
Renames reps→times, mode→directed, and adds .directed_label() helper. |
R/qap_engine.R |
Replaces legacy engines with unified QAPengine() + QAPPermEst() and permute scheme support. |
R/qap_css.R |
Removes duplicated CSS engine; keeps CSS vectoriser + print.QAPCSS and updates to new vocabulary. |
R/model_tests.R |
Standardizes network_test return fields to directed/times. |
R/model_regression.R |
Switches net_regression() to QAPengine(), adds default-resolution reporting, and updates printing for new vocabulary. |
R/qap_gpu.R |
Removes torch-based GPU permutation shortcut implementation. |
R/qap_gmm.R |
Removes GMM moment/residual helpers along with the GMM estimator path. |
NEWS.md |
Adds v0.2.0 release notes covering the refactor, removals, and new tests. |
NAMESPACE |
Registers print.QAPCSS S3 method. |
man/regression.Rd |
Updates documentation to the new control vocabulary and reduced feature set. |
DESCRIPTION |
Bumps version to 0.2.0 and reduces Suggests dependencies. |
.github/CONTRIBUTING.md |
Documents standardized vocabulary and lists parked extensions/branches. |
Review details
Files not reviewed (1)
- man/regression.Rd: Generated file
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Package
Suggestspackages from eight to threeRegression
net_regression()failing on a two-mode network with more columns thanrows (closes net_regression not working on large, weighted two-mode network, failing without informative error message #4)
NAand the dyad count came back as
NAreps=totimes=including on the returned fitmethod=/nullhyp=topermute=, as method can be ambiguousmethod = "qap"/nullhyp = "qapspp"is nowpermute = "predictor"method = "qapy"is nowpermute = "outcome"mode=todirected=, reserving mode for one-mode and two-mode networksmode = "undirected"is nowdirected = FALSEdatais retired as potentially confusing:.dataremains the network the user passes inmatlistis the named list of matrices the engine fitsnetis one coerced network, inside the formula front endsnet_info()reporting of every default the model resolves for itselfpermute = "predictor"falling back to"outcome"with one predictorQAPglm()andQAPcss()engines into one,QAPengine()returning residuals, and identifying random intercepts
torchGPU path (feature/torch-gpu)CUDA device;
{torch}in Suggests broke the CI buildgmmestimator and theestimatorcontrol (feature/gmm-estimator)family, on well-conditioned data
feature/glmmtmb-mixed){glmmTMB}carries 62 recursive dependencies and must match{TMB}negbinandzippaths are unaffectedfamily = "multinom"and thecomparison/referencecontrols(
feature/multinomial-comparison)engines at 21 points
fixest_se_clustercontrol (feature/fixest-fixed-effects){lme4}random-effect term, andnothing else;
parse_qap_formula()drops from three branches to oneTests
test-qap_reporting.R, which runs withsnet_verbosity = "verbose"test-qap_shape_css.R, which fits a cognitive social structure