squidgroup / squidsim_vignette Goto Github PK

Need to be more specific about nesting, crossing and interactions - what make_structure actually does, and what limitations it has

More figure such as for the nested structure would help. And maybe then export the function to make them

Section 1.8. Why is it sometimes useful to not scale the predictors by their beta values (by adding I)? When is it useful to add an extra beta to a predictor that already has a beta value? These options are introduced as part of the simulate_population function, but to me it is not entirely clear when one would want to do that. I think a brief explanation and perhaps an example would be helpful. The vignette is very easy to read and straightforward, but here my reading flow got interrupted.

Here some feedback for the squidSim vignette:

1. I am unsure why the name of the package {squidSim} is written in brackets. Can these not be removed?
2. Under "why use squidSim" the text sounds slightly condescending. I suggest to remove the first two sentences.
3. Under "why use squidSim" there are various points. Perhaps these can be formatted in a numbered list instead?
4. Under "Issues and bugs" there is a space missing (herefor isues)
   Simple linear model
5. Equations are part of a sentence, and need to be integrated as such with commas and dots. For example, the first equation under "Simple Linear Model" needs a comma after the residual, but this should be checked throughout the vignette.
6. Under the first equation it reads "and some unexplained, residual variation". Remove either unexplained or residual, as they mean the same thing.
7. Eq. 2 lacks a transpose on x to indicate that it is a row vector so that x_i^t \beta is an inner product
8. Under the column vector for beta, correct to "We will use this notation throughout the vignette"
9. "If we want to simulate from this model, we can assume.." correct to "we assume" - you can assume the predictor is generated from whatever distribution, but you specifically assume it follows from a normal distribution.
10. "Parameters for the residual must be specified, all other levels are optional". I suppose this implies defaults used by squidSim, can those be mentioned here?
11. "First we can specify or sample size" -> remove "can"'
12. "We can also give the response (y) ...". The response should be either $y_i$ or $\bm{y}$ following notation earlier in this page
13. "or in words" remove "observation level" in the next sentence - it does not add anything
14. "For the observation list.." after this follows the names for the predictors, and the vignette is not consistent here. Previously a name is written as inline code, but here it is just as ordinary text. Please correct and be consistent throughout
15. "Let's compare the code back to the model", the first line of the equation colors terms and so does the code chunk, but the two other lines in the equation are not colored accordingly. Can this be edited?
16. "Its worth noting" -> "It's worth noting"
17. "the function assumes the covariances are 0" write out integers under ten?
    Correlated predictors
18. "Matrices in R" is colored orange - unclear why
19. "which can be changed to per row, by specifying the "byrow=T" argument
    Interactions and non-linear effects
20. First equation here separates the subscript on the x's with a comma, but in previous equations that was not the case. Again, correct for consistency throughout please
21. "Interactions can be between predictors or at the same or different hierarchical level" difficult running sentence, perhaps correct to "Interactions can be specified between two predictors at the same level, or at different hierarchical levels" or similar
    Transformations
22. "before they are scaled by the beta values". Predictors are not scaled by the slopes, but multiplied with. Please correct throughout.
23. Histogram for data$rainfall, maybe give this nicer axis labels and a title?
24. "The simulate covariance" -> simulated covariance?
    Known predictors
25. "we might have the situation where.." can be more concise -> "We might instead want to use existing predictors (e.g., that follow another distribution)"
26. I would advice not referring to MCMCglmm so much - there are other datasets available in less installation heavy R-packages, or in base R that you can use instead.
    Non-Gaussian phenotypes
27. simulate_population function should be inline code
28. "POisson" -> correct
29. Please do not use \hat{y} to mean the mean. The hat symbol is most often used to indicated a predicted or estimated value, so this will be confusing to a lot of people. Just call it lambda instead (as is usual in Poisson regression)
30. \epsilon_i is all of a sudden \epsilon_1 one line further, correct please
    Model equations
31. Again, simulate_population should be inline code.

http://squidgroup.org/squidSim_vignette/2.4-randomslopes.html includes an "interactions" argument in the simulations, yet there is no interaction included in the model. This term seems to be used to set-up random-slope model, which includes a multiplication of a parameter vector, and a predictor, which is not an "interaction" as in the classical fixed-effects sense. Thus, the "interaction" terminology might be confusing.

• Wrong package name:

  squidSim_vignette/07-temporal.Rmd

  Line 86 in 0ff82e7

  `squidR` uses the sinusoidal equation to implement cyclical temporal effects

• Missing space after second 'here':

  squidSim_vignette/index.Rmd

  Line 97 in 0ff82e7

  It would be great if you could report any suggestions, issues or bugs; [here](https://github.com/squidgroup/squidSim/issues) for issues relating to the package, and [here](https://github.com/squidgroup/squidSim_vignette/issues)for issues relating to this vignette. It is worth checking to see if anyone else has a similar problem first, and adding comments to their issue, before starting a new one.

• 'POisson':

  squidSim_vignette/02-sim_pop.Rmd

  Line 669 in 0ff82e7

  Here is an example to simulate POisson distributed data:

• 'that is need is':

  squidSim_vignette/02-sim_pop.Rmd

  Line 686 in 0ff82e7

  The only change in the code that is need is the addition of the link and family arguments.

• 'is always need':

  squidSim_vignette/02-sim_pop.Rmd

  Line 846 in 0ff82e7

  The parameters list contains one (or more) list for each hierarchical level that you want to simulate at. A residual list is always need, specifying variances/covariances for the residual. Additionally, the parameter list can also be provided with an intercept vector and interactions list.

• 'paramter':

  squidSim_vignette/02-sim_pop.Rmd

  Line 848 in 0ff82e7

  The simplest paramter list will look something like this:

At the moment predictors can vary at the individual or observation level. But in many instances the same predictor vary at both levels. We can consider allowing the same predictor to vary at both levels. With different effects at each level. In a way we could simulate among and within individual variation in the predictors, as we simulate among and within individual variation in the response due to unknown sources. I also found a bit confusing when the cov argument refer to the predictors and when directly to the phenotypes.

While reading the vignette, I noticed that the residual variable in the output data is sometimes named residual and other times residual_effect. It should have the same name for all simulations to avoid confusion.

Relating to this, in the section 6.2 of the vignette, this code returns an error because residual_effect was used for the model formula instead of residual.

squid_data <- simulate_population(
  parameters=list(
    year_cont = list(
      group="year",
      names= "linear_effect",
      covariate=TRUE,
      beta=0.3,
      amplitude = 2,       # |A| = the amplitude
      period = 10,
      h_shift = 3,
      v_shift = 5
    ),
    year = list(
      vcov = 1.2
    ),
    residual=list(
      vcov = 1
    )
  ), 
  data_structure= make_structure(structure = "year(20) + sex(2)/individual(50)",N=1000),
  
  model=" B =(2*pi) / abs(period);
          cyclic_effect = amplitude*sin(B*I(linear_effect) - B^2*h_shift ) + v_shift;
          y = linear_effect + cyclic_effect + year_effect + residual_effect"
)

In Mathematical Notation -> Interactions / Random regression and Multi-response

Can take the exercises from the workshop and add in to vignette. Can have on own page utilise the code hiding part to hide solution code, or have solution on seperate page