Relax Distributions compat requirement - #14
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| current_pos = n_fixed | ||
| for effect in model.random_effects | ||
| name = effect.name | ||
| Z = Z_dict[name] | ||
| dim = size(Z, 2) |
There was a problem hiding this comment.
Guard against missing design matrices for unsupported random effects
Random effects marked as “ignored” in build_design_matrices are not inserted into Z_dict, yet the MME builder still iterates over every entry in model.random_effects and unconditionally accesses Z_dict[name]. When the model includes an unimplemented type (e.g. RandomEffect("herd", :iid)), this line throws a KeyError before the evaluation even begins, contrary to the warning that the effect will be skipped. The loop should skip or stub out unsupported effects so setup_mme remains executable.
Useful? React with 👍 / 👎.
| # 更新残差方差 | ||
| residuals = y - X * solutions[1:n_fixed] - sum(Z * solutions[n_fixed+1:end] for (name, Z) in Z_dict) | ||
| s_y = dot(residuals, residuals) | ||
| tr_term = tr(C_inv[1:n_fixed, 1:n_fixed] * (X' * X)) # 简化 | ||
| variances_new["residual"] = (s_y + tr_term) / n_obs | ||
|
|
||
| # 更新随机效应方差 | ||
| current_pos = n_fixed | ||
| for effect in model.random_effects | ||
| name = effect.name | ||
| dim = size(Z_dict[name], 2) | ||
| u = solutions[current_pos+1 : current_pos+dim] | ||
|
|
||
| if effect.type == :additive && !isnothing(dm.A_inv_matrix) | ||
| u_Ainv_u = dot(u, dm.A_inv_matrix * u) | ||
| tr_term_u = tr(C_inv[current_pos+1:end, current_pos+1:end] * dm.A_inv_matrix) # 简化 | ||
| variances_new[name] = (u_Ainv_u + tr_term_u) / dim |
There was a problem hiding this comment.
REML update uses full random vector for each effect
In the REML iteration, each random-effect design matrix is multiplied by solutions[n_fixed+1:end] and the trace term slices C_inv[current_pos+1:end, current_pos+1:end]. These expressions assume only one random effect: with two or more effects, solutions[n_fixed+1:end] stacks all effects so Z * … has mismatched dimensions and the trace calculation mixes unrelated blocks, causing dimension errors or incorrect variance updates. The residual and trace calculations need to use the segment of the solution and inverse matrix corresponding to the current effect (current_pos+1:current_pos+dim).
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68de4933440c8321821480b658d82ad6