conroylau / lpinfer

lpinfer: An R Package for Inference in Linear Programs
GNU General Public License v3.0
3 stars 5 forks source link

invertci bracketing is confusing #98

Closed a-torgovitsky closed 4 years ago

a-torgovitsky commented 4 years ago

This is what is written in the README.md:

lb0 refers to the logical lower bound for the confidence interval.
lb1 refers to the maximum possible lower bound for the confidence interval.
ub0 refers to the logical upper bound for the confidence interval.
ub1 refers to the minimum possible upper bound for the confidence interval.

Let's take a step back here.

What we want here is for the user to be able to specify an initial bracket. This has nothing to do with the "logical lower bound" or "maximum possible lower bound." We use the logical lower bound if the user does not pass one only because that's our best guess.

I think we need to re-design this to be more clear by removing these four options and replacing them with:

These are just initial brackets. If the user passes initial brackets, then we use them. If they don't, then we continue doing what we are doing now and infer them.

conroylau commented 4 years ago

Done! Just updated the invertci function with init.lb and init.ub that represent the initial brackets.

Do you think the following assignments are appropriate if init.lb and init.ub are not both vectors? Scenario lb initial bracket assignment ub initial bracket assignment
init.lb = [a, b] and init.ub is a scalar [a, b] [a, init.ub]
init.lb is a scalar and init.ub = [c, d] [init.lb, d] [c, d]
init.lb and init.ub are scalars [init.lb, init.ub] [init.lb, init.ub]
init.lb is a scalar and init.ub is NULL [init.lb, logical.ub] [init.lb, logical.ub]
init.lb is NULL and init.ub is a scalar [logical.lb, init.ub] [logical.lb, init.ub]
init.lb and init.ub are both NULL [logical.lb, logical.ub] [logical.lb, logical.ub]

(where logical.lb and logical.ub are the logical lower and upper bounds respectively).

In addition, as mentioned in #79, when chorussell is used in invertci, the confidence intervals are constructed using the chorussell procedure directly. Hence, init.lb and init.ub will be ignored if f = chorussell in invertci.

On the other hand, I have also updated the sample code in the example folder to reflect the change.

Thanks!

a-torgovitsky commented 4 years ago

Do you think the following assignments are appropriate if init.lb and init.ub are not both vectors?

I think that's too complicated and I can't imagine relevant use cases where any of the first five rows would be important. The only thing we want to accept from the user for each of init.lb and init.ub is the default (NULL) or a vector of two elements.

By the way, what happens currently if the logical.lb or logical.ub is turns out to be +/- Infinity? In this case I think we should error and tell the user that they need to pass init.lb and init.ub explicitly.

conroylau commented 4 years ago

Just updated the invertci procedure:

Scenario lb initial bracket assignment ub initial bracket assignment
init.lb = [a, b] and init.ub is NULL [a, b] [a, logical.ub]
init.lb is NULL and init.ub = [c, d] [logical.lb, d] [c, d]

Thanks!

a-torgovitsky commented 4 years ago

Yes I think that's reasonable. Thanks!