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

clean up simulations #456

Merged
merged 10 commits into from
Aug 5, 2024
Merged

clean up simulations #456

merged 10 commits into from
Aug 5, 2024

Conversation

clarkliming
Copy link
Collaborator

@clarkliming clarkliming commented Jul 26, 2024

there are just too many updates done within this merge request, @danielinteractive please have a careful review.

The reason to have this is to achieve smooth simulation. The changes (and reasons) are listed as below

  1. totally remove simChef usage.
    1. current structure of using simChef is not efficient, because all model objects are saved. with large dataset, a tmb model can be over 1GB size. it is impossible to store all these models without crashing.
    2. we don't really need the simChef simulation framework. simpler simulations easier to configure
  2. remove sasr usage.
    1. sasr with ssh tunnels is not stable enough for large simulations
  3. fix convergence criteria.
    1. previously for mmrm it only check if there is an error but not the convergence code
  4. ML and REML are both included in the simulation
    1. previously glmmTMB is using ML instead of REML, but for other simulations they all use REML
  5. missing data pattern is updated
    1. previously only in extreme missing scenario the missing is related to treatment. to have more consistency the treatment is not affecting the missing pattern now
  6. simulation for nlme::gls is updated
    1. previously for nlme::gls, the covariance structure is not correct. it should be ~ visit|id
    2. the toeplitz covariance structure is also added
  7. simulation data updated
    1. now for the same configuration but only different covariance structure in data generation, all the other covariates are the same. the only different thing is the error term
    2. now the toeplitz covariance structure has the same variance (toep, us
  8. improve the covariance structure
    1. previously for gls and sas, the covariance structure for the first subject is returned. now it is re-constructed and no matter if there is anyone with full visits, a full covariance structure will be returned.

@clarkliming clarkliming marked this pull request as draft July 26, 2024 01:10
Copy link
Contributor

github-actions bot commented Jul 26, 2024

badge

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       3  98.69%   420, 481, 511
R/interop-car.R                72       3  95.83%   9, 48, 68
R/interop-emmeans.R            51       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              140       0  100.00%
R/residual.R                    8       0  100.00%
R/satterthwaite.R             116      12  89.66%   238-249
R/skipping.R                    8       0  100.00%
R/testing.R                    64       4  93.75%   29, 31, 80, 82
R/tidiers.R                    72       2  97.22%   46-47
R/tmb-methods.R               281       3  98.93%   278-279, 339
R/tmb.R                       283       0  100.00%
R/utils-formula.R              27       0  100.00%
R/utils-nse.R                  16       0  100.00%
R/utils.R                     193      12  93.78%   279-289, 485, 514
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                    78       0  100.00%
TOTAL                        3269      98  97.00%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: 0b08bd6

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@danielinteractive
Copy link
Collaborator

Apologies @clarkliming for the delayed review, will do this tomorrow

Copy link
Collaborator

@danielinteractive danielinteractive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks so much simpler. Thanks a lot @clarkliming !!

@danielinteractive danielinteractive marked this pull request as ready for review July 31, 2024 14:02
Copy link
Contributor

github-actions bot commented Aug 5, 2024

Unit Tests Summary

    1 files     46 suites   23s ⏱️
  499 tests   461 ✅ 38 💤 0 ❌
1 908 runs  1 865 ✅ 43 💤 0 ❌

Results for commit 0b08bd6.

@clarkliming clarkliming merged commit 424810d into main Aug 5, 2024
25 of 26 checks passed
@clarkliming clarkliming deleted the mmrm_missing_simulations branch August 5, 2024 08:42
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