Improve relationship matrices and data alignment - #9
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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
| # --- E-步: 求解MME --- | ||
| C, rhs = setup_mme(X, Z_dict, y, dm, model, variances, dm.animal_map) | ||
| C_inv = inv(Matrix(C)) # 警告: 这是计算瓶颈，更高级的AI-REML会避免它 |
There was a problem hiding this comment.
The call to setup_mme passes dm.animal_map as a seventh argument, but setup_mme is defined to accept only six parameters. As written this function will throw a MethodError before any REML iteration starts. Dropping the extra argument (or extending setup_mme to accept it) is required for estimate_variances_reml to run.
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.
During the M-step the code subtracts random-effect contributions and computes u'A⁻¹u using solutions[n_fixed+1:end] for every random effect. When more than one random effect is present, Z * solutions[n_fixed+1:end] and C_inv[current_pos+1:end, current_pos+1:end] use the entire concatenated random-effect vector rather than the block corresponding to the current effect, leading to dimension mismatches or incorrect variance updates. The residual and trace computations should slice solutions and C_inv using current_pos+1:current_pos+dim for each effect.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68dde500f57c83218f47ce973c738cbe