benbhansen-stats / propertee

Prognostic Regression Offsets with Propagation of ERrors, for Treatment Effect Estimation (IES R305D210029).
https://benbhansen-stats.github.io/propertee/
Other
2 stars 0 forks source link

Uninformative error #100

Closed josherrickson closed 1 year ago

josherrickson commented 1 year ago
> lm(y ~ assigned(des), data = simdata, weights = ate())
Error in .get_design() : 
  Unable to locate Design in call stack, please use the  `design` argument to pass a Design object.
> lm(y ~ assigned(des), data = simdata, weights = ate(), 
   offset = cov_adj(cmod, design = des))

Call:
lm(formula = y ~ assigned(des), data = simdata, weights = ate(), 
    offset = cov_adj(cmod, design = des))

Coefficients:
  (Intercept)  assigned(des)  
      0.08263       -0.16138  

> lm(y ~ assigned(des), data = simdata, weights = ate(), 
   offset = cov_adj(cmod))
Error in get(type, sys.frame(i)) : 
  promise already under evaluation: recursive default argument reference or earlier problems?

If ate() is called in lm() without passing a Design and no offset, it errors appropriately.

if ate() is called and cov_adj() properly has a design= argument, it works as expected.

If ate() is called and cov_adj() does not have a design= argument it gives a nonsense error message.

benthestatistician commented 1 year ago

Bummer. But good catch!

If cov_adj() is going to need its design= argument regularly, perhaps we should promote it from third in the order of args to second, so that users can abbreviate cov_adj(cmod, design=des) to cov_adj(cmod, des).

josherrickson commented 1 year ago

We need at least one of ate()/ett() or cov_adj() to have a design= argument, not both. In the 2x2 table of weights yes/no and cov_adj yes/no, if we expect the weights no/cov_adj yes box to have a high proportion of all mdoels, then perhaps that would make sense.

Just to be clear, this issue isn't saying that we need to pass Design to both weights and cov_adj; just that when both are present and no design= argument is passed, the error is inappropriate.

benthestatistician commented 1 year ago

Got it, thanks! I'll propose that we take any further followups about the cov_adj() UI over to #4.