Skip to content

Commit

Permalink
fix crash in demeaning when nobs = 1, fixes: #283
Browse files Browse the repository at this point in the history
  • Loading branch information
lrberge committed Feb 9, 2024
1 parent 06ce3b5 commit dc40b58
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 16 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
- fix bug in the `collinearity` function. Reported by @grlju, #412

- fix bug which allowed the estimation of models with variables from the environment while it shouldn't

- fix bug in the demeaning code when the data set in input is of length 1. Reported by @da-zar with help of @etiennebacher, #283

## Improvements

Expand Down
17 changes: 8 additions & 9 deletions R/estimation.R
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,9 @@ feols = function(fml, data, vcov, weights, offset, subset, split, fsplit, split.
#

onlyFixef = length(X) == 1 || ncol(X) == 0
if(length(y) == 1){
onlyFixef = is.null(dim(X)) || ncol(X) == 0
}

if(fromGLM){
res = list(coefficients = NA)
Expand Down Expand Up @@ -1671,7 +1674,7 @@ feols = function(fml, data, vcov, weights, offset, subset, split, fsplit, split.

res$convStatus = FALSE

res$message = paste0("tol: ", signif_plus(fixef.tol), ", iter: ", max(res$iterations))
res$message = paste0("tol: ", fsignif(fixef.tol), ", iter: ", max(res$iterations))

if(fromGLM){
res$warn_varying_slope = msg
Expand Down Expand Up @@ -1719,7 +1722,7 @@ feols = function(fml, data, vcov, weights, offset, subset, split, fsplit, split.
if(mem.clean){
gc()
}

if(!onlyFixef){

est = ols_fit(y_demean, X_demean, weights, correct_0w, collin.tol, nthreads,
Expand All @@ -1739,13 +1742,9 @@ feols = function(fml, data, vcov, weights, offset, subset, split, fsplit, split.
all_vars = colnames(X)

if(isFixef){
msg = paste0(ifsingle(all_vars, "The only variable ", "All variables"),
enumerate_items(all_vars, "quote.is", nmax = 3),
" collinear with the fixed effects. In such circumstances, the estimation is void.")
msg = sma("{$(The only variable;All variables)}, {enum.q.3 ? all_vars}, {$are} collinear with the fixed effects. In such circumstances, the estimation is void.")
} else {
msg = paste0(ifsingle(all_vars, "The only variable ", "All variables"),
enumerate_items(all_vars, "quote.is", nmax = 3),
" virtually constant and equal to 0. In such circumstances, the estimation is void.")
msg = sma("{$(The only variable;All variables)}, {enum.q.3 ? all_vars}, {$are} virtually constant and equal to 0. In such circumstances, the estimation is void.")
}

if(IN_MULTI || !warn){
Expand All @@ -1761,7 +1760,7 @@ feols = function(fml, data, vcov, weights, offset, subset, split, fsplit, split.
return(fixest_NA_results(env))

} else {
stop_up(msg, up = fromGLM)
stop_up(msg, up = 1 * fromGLM)
}


Expand Down
28 changes: 21 additions & 7 deletions src/demeaning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class sMat{
sMat() = delete;

public:
sMat(SEXP);
sMat(SEXP, bool);

int nrow(){return n;};
int ncol(){return K;};
Expand All @@ -165,7 +165,7 @@ class sMat{
double operator()(int, int);
};

sMat::sMat(SEXP x){
sMat::sMat(SEXP x, bool single_obs = false){

if(TYPEOF(x) == VECSXP){
// x can be a list of either vectors or matrices
Expand Down Expand Up @@ -231,7 +231,7 @@ sMat::sMat(SEXP x){
K = pdim[1];
}

if(n == 1 && K == 1){
if(!single_obs && (n == 1 && K == 1)){
// => absence of data
n = 0;
K = 0;
Expand Down Expand Up @@ -1797,8 +1797,8 @@ void stayIdleCheckingInterrupt(bool *stopnow, vector<int> &jobdone, int n_vars,
// Loop over demean_single
// [[Rcpp::export]]
List cpp_demean(SEXP y, SEXP X_raw, SEXP r_weights, int iterMax, double diffMax, SEXP r_nb_id_Q,
SEXP fe_id_list, SEXP table_id_I, SEXP slope_flag_Q, SEXP slope_vars_list,
SEXP r_init, int nthreads, bool save_fixef = false){
SEXP fe_id_list, SEXP table_id_I, SEXP slope_flag_Q, SEXP slope_vars_list,
SEXP r_init, int nthreads, bool save_fixef = false){
// main fun that calls demean_single
// preformats all the information needed on the fixed-effects
// y: the dependent variable
Expand Down Expand Up @@ -1827,6 +1827,20 @@ List cpp_demean(SEXP y, SEXP X_raw, SEXP r_weights, int iterMax, double diffMax,
n_obs = m_X.nrow();
}
bool useX = n_vars_X > 0;

if(n_obs == 0 || n_obs == 1){
// The data set is of length 1!!!!
n_obs = 1;

m_y = sMat(y, true);
n_vars_y = m_y.ncol();
useY = true;

m_X = sMat(X_raw, true);
n_vars_X = m_X.ncol();
useX = n_vars_X > 0;

}

int n_vars = n_vars_y + n_vars_X;

Expand All @@ -1836,7 +1850,8 @@ List cpp_demean(SEXP y, SEXP X_raw, SEXP r_weights, int iterMax, double diffMax,
bool saveInit = ((isInit || init[0] != 0) && Q > 1) || init[0] == 666;

// Creating the object containing all information on the FEs
FEClass FE_info(n_obs, Q, r_weights, fe_id_list, r_nb_id_Q, table_id_I, slope_flag_Q, slope_vars_list);
FEClass FE_info(n_obs, Q, r_weights, fe_id_list, r_nb_id_Q, table_id_I, slope_flag_Q,
slope_vars_list);
int nb_coef_T = FE_info.nb_coef_T;

// output vector: (Note that if the means are provided, we use that vector and will modify it in place)
Expand Down Expand Up @@ -1971,7 +1986,6 @@ List cpp_demean(SEXP y, SEXP X_raw, SEXP r_weights, int iterMax, double diffMax,
res["X_demean"] = X_demean;



if(is_y_list && useY){
List y_demean(n_vars_y);

Expand Down
5 changes: 5 additions & 0 deletions tests/fixest_tests.R
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ test(feols(y ~ x1 | factor(fe1^fe2), base), "err")

res = feols(y ~ x1 | bin(x2, "bin::1")^fe1 + fe1^fe2, base)

# 1 obs (after FE removal) estimation
base = data.frame(y = c(1, 0), fe = c(1, 2), x = c(1, 0))
test(fepois(y ~ x | fe, base), "err")
# no error
res = fepois(y ~ 1 | fe, base)



Expand Down

0 comments on commit dc40b58

Please sign in to comment.