Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve an issue with starting betas being reset #60

Merged
merged 2 commits into from
May 31, 2023

Conversation

lhjohn
Copy link
Contributor

@lhjohn lhjohn commented May 20, 2022

@egillax and I started implementing #59 and came across a bug. During cross validation the beta values are reset to 0.0, while they should be reset to the starting betas, if provided.

PR has the following additions.

  • Add local variable to hold starting coefficients.
  • Reset betas during CV to starting coefficients, if provided.
  • Add some getters to the interface useful for debugging

Code to reproduce initial problem providing useCrossValidation = TRUE, startingCoefficients and fixedCoefficients :

library(Cyclops)

set.seed(1)
nCovars <- 10

# simulate the data
simulant <- simulateCyclopsData(nstrata = 1,
                                nrows = 5000,
                                ncovars = nCovars,
                                model = "logistic")

targetData <- convertToCyclopsData(simulant$outcomes,
                                   simulant$covariates,
                                   modelType = "lr")

# fix all coefficients, but not the intercept
fixedCoef <- c(FALSE,
               rep(TRUE, nCovars))

# initialize a vector for starting coefficients to 0.5
startingCoef <- rep(0.5, nCovars+1)

# useCrossValidation = FALSE
fit <- fitCyclopsModel(targetData,
                       prior = createPrior("laplace",
                                           variance = 0.1,
                                           useCrossValidation = FALSE),
                       forceNewObject = TRUE,
                       fixedCoefficients = fixedCoef,
                       startingCoefficients = startingCoef)

# store coefficients
fitCoef <- coef(fit)

# print coefficients without cross validation: starting value is taken into account
fitCoef

# useCrossValidation = TRUE
fit2 <- fitCyclopsModel(targetData,
                        prior = createPrior("laplace",
                                            variance = 0.1,
                                            useCrossValidation = TRUE),
                        forceNewObject = TRUE,
                        fixedCoefficients = fixedCoef,
                        startingCoefficients = startingCoef)

# store coefficients
fit2Coef <- coef(fit2)

# print coefficients with cross validation: starting value is not taken into account
fit2Coef

During cross validation the beta values are reset to 0.0, while
they should be reset to the starting betas, if provided.
@lhjohn
Copy link
Contributor Author

lhjohn commented Sep 22, 2022

@msuchard Could you have a look at this issue? This bug prevents us from implementing transfer learning using priors. I'd be happy to submit a revised version if you have some feedback.

@msuchard
Copy link
Member

@lhjohn -- did this get fixed already?

@lhjohn
Copy link
Contributor Author

lhjohn commented May 23, 2023

@msuchard this problem persists. Could you run the code snippet on your end to show that when using cross validation the starting coefficients are ignored? This is caused by beta values resetting to 0.f for the folds. I could provide a more minimal solution if that helps.

@lhjohn lhjohn changed the base branch from main to develop May 23, 2023 08:45
@msuchard msuchard merged commit 8597a68 into OHDSI:develop May 31, 2023
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.

2 participants