Skip to content

Conversation

@kgreenewald
Copy link

No description provided.

Updates to improve robustness and quality-of-life:
-Addition of a max_norm constraint to avoid overflow/nonsparse outcomes given the nonconvex regularizer (involves a minor change to the optimization algorithm). This was a parameter that the theory had included, but we had omitted this from the original version since it only seems to matter in extreme circumstances where the hyperparameters are poorly chosen. There is also an auto option here for easy use, as well as none to ignore.
-Addition of an auto option for the mcp_alpha parameter, since this may be somewhat mysterious to some.
-Addition of an include_bias option defaulted to True. This subtracts off the mean of each group of covariates and outcomes (something we typically did outside the module), which is equivalent to adding an unregularized bias term to the regressions.
-The replacement of the auto option for mcp_lambda with an option proportional_lambda, which, if set to True, interprets the value of mcp_lambda as a constant of proportionality to the previous auto option. This is because I found in practice, the coefficient of the auto option pretty much always needs to be tuned, but that the analytic expression it uses was extremely helpful otherwise.
edit parameter description
@CLAassistant
Copy link

CLAassistant commented May 31, 2023

CLA assistant check
All committers have signed the CLA.

ehudkr and others added 5 commits July 29, 2024 11:23
Commit d52d8b2 (and 855c11f) removed support of `lmda="auto"`
in favor of `proportional_lambda` and `alpha` auto settings.
Fixes a bug where `proportional_lambda` was always set to `True`
in `MCPSelector` regardless of the value passed to the actual
user-instantiated `SharedSparsityConfounderSelection` model.
@ehudkr
Copy link
Member

ehudkr commented Jul 30, 2024

Apologies for the late response, I was under the impression everything was ok and just waited for the right version change to merge and release. However, it sems these changes broke the existing tests.
I was able to alleviate some of the more technical ones, but it seems current behavior selects all possible covariates in the tests, where according to the data generating process, only 2 should be selected.
I couldn't replicate the model parameters from the before-change in order to see if they reproduce past behavior and pass the test. I also tinkered with some of them (but nothing too comprehensive) but was not able to make it align with past behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants