enable other start values

[clarkliming]

close #387

[github-actions]

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  ----------------------------
R/between-within.R             59       0  100.00%
R/component.R                  67       0  100.00%
R/cov_struct.R                 97       1  98.97%   407
R/empirical.R                   7       0  100.00%
R/fit.R                       229       7  96.94%   186-189, 420, 481, 511
R/interop-car.R                70       2  97.14%   9, 48
R/interop-emmeans.R            39       0  100.00%
R/interop-parsnip.R            59       1  98.31%   12
R/kenwardroger.R               92       2  97.83%   41, 63
R/mmrm-methods.R              120       0  100.00%
R/residual.R                    8       8  0.00%    10-31
R/satterthwaite.R             116      12  89.66%   238-249
R/testing.R                    64       4  93.75%   29, 31, 80, 82
R/tidiers.R                    72       2  97.22%   46-47
R/tmb-methods.R               287       3  98.95%   277-278, 338
R/tmb.R                       281       0  100.00%
R/utils-formula.R              27       0  100.00%
R/utils-nse.R                  16       0  100.00%
R/utils.R                     184      12  93.48%   276-286, 456, 485
R/zzz.R                        70      24  65.71%   7-22, 55-60, 90, 118, 138
src/chol_cache.h               63       0  100.00%
src/covariance.h              101       1  99.01%   177
src/derivatives.h             126       0  100.00%
src/empirical.cpp              72       0  100.00%
src/exports.cpp                47       0  100.00%
src/jacobian.cpp               47       1  97.87%   54
src/kr_comp.cpp                56       0  100.00%
src/mmrm.cpp                   76       0  100.00%
src/predict.cpp                93       0  100.00%
src/test-chol_cache.cpp        58       5  91.38%   9, 18, 26, 55, 62
src/test-covariance.cpp       123       5  95.93%   9, 29, 40, 61, 72
src/test-derivatives.cpp      108       7  93.52%   44, 53, 62, 85, 94, 106, 124
src/test-utils.cpp            195       7  96.41%   9, 16, 24, 34, 44, 57, 119
src/testthat-helpers.h         15       5  66.67%   36-37, 41, 50, 53
src/utils.h                    84       0  100.00%
TOTAL                        3228     109  96.62%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
R/fit.R          +6      -4  +1.88%
R/tmb.R          -6       0  +100.00%
R/utils.R       +50      +1  +1.69%
TOTAL           +50      -3  +0.15%

Results for commit: 1cf9e9e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

    1 files     44 suites   25s ⏱️
  477 tests   439 ✅ 38 💤 0 ❌
1 859 runs  1 816 ✅ 43 💤 0 ❌

Results for commit 1cf9e9e.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
fit	👶		$+0.04$	mmrm_fails_if_start_function_does_not_provide_correct_values
fit	👶		$+0.02$	mmrm_fails_if_start_function_fails
fit	👶		$+0.19$	mmrm_works_for_character_covariate_using_emp_start
fit	👶		$+0.06$	mmrm_works_for_start_NULL
fit	👶		$+0.25$	mmrm_works_for_transformed_response_using_emp_start
fit	👶		$+0.18$	start_can_be_emp_start_or_std_start_and_they_work
utils	👶		$+0.01$	emp_start_works
utils	👶		$+0.01$	h_get_theta_from_cov_use_0_1_to_impute_the_NA_values
utils	👶		$+0.01$	h_get_theta_from_cov_works
utils	👶		$+0.04$	std_start_works

Results for commit b0144f0

♻️ This comment has been updated with latest results.

[clarkliming]

with different starting values, there is some minor changes to all the current tests. The difference is small and it should be fine.

Another possibility is that we add start = "std_start" in all tests so the result will not be impacted.

another thing is that I use std_start (standard start) because it is a little different from the ols start values. from SAS guide it says

requests starting values corresponding to the usual general linear model. Specifically, all variances
and covariances are set to zero except for the residual variance, which is set equal to its ordinary least
squares (OLS) estimate. This option is useful when the default MIVQUE0 procedure produces poor
starting values for the optimization process.

all variance and covariance are set to 0 except the residual variance. it is closer to our emp_start?

[danielinteractive]

Nice work thanks @clarkliming !

Just a few ideas before diving into detailed review:

• Did you already add an integration test that resembles the reprex example that motivates this?
• Yes agree that those minor test result differences are ok so ok to update tolerances
• Is our std start not always giving an identity covariance matrix to start with? If yes this should resemble the OLS as first iteration?

[clarkliming]

Nice work thanks @clarkliming !

Just a few ideas before diving into detailed review:

• Did you already add an integration test that resembles the reprex example that motivates this?

I will add it now. the example is a bit complicated

• Yes agree that those minor test result differences are ok so ok to update tolerances

• Is our std start not always giving an identity covariance matrix to start with? If yes this should resemble the OLS as first iteration?

for AR1 and AR1H, the $\rho$ are set to 0.5 by default, otherwise there are convergence issues for nlminb optimizer

fit_single_optimizer(
    formula = FEV1 ~ ar1(AVISIT | USUBJID),
    data = fev_data,
    weights = rep(1, nrow(fev_data)),
    optimizer = "nlminb",
    start = c(0, 0.5) # update to c(0,0) that errors
  )

[clarkliming]

with some extra cases, using empirical values can be worse

fit <- mmrm(FEV1 ~ AVISIT * ARMCD + us(AVISIT | USUBJID), data = fev_data)
  full_frame <- fit$tmb_data$full_frame
  sig_scale <- c(VIS1 = -4, VIS2 = -8, VIS3 = 8, VIS4 = 12)
  full_frame$FEV1 <- fitted(fit) + residuals(fit) * exp(sig_scale[full_frame$AVISIT])
  fit2 <- fit_mmrm(
    FEV1 ~ AVISIT * ARMCD + us(AVISIT | USUBJID),
    data = full_frame, weights = rep(1, nrow(full_frame)),
    control = mmrm_control(start = "std_start")
  )
  fit2 <- fit_mmrm(
    FEV1 ~ AVISIT * ARMCD + us(AVISIT | USUBJID),
    data = full_frame, weights = rep(1, nrow(full_frame)),
    control = mmrm_control(start = "emp_start")
  )

using emp_start gives worse result.