Fix AnimalBreeding Project.toml metadata - #13
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
| rename_map = Dict{String,String}() | ||
| for (std_name, aliases) in required_cols | ||
| col_found = findfirst(name -> name in names(pedigree), aliases) | ||
| if isnothing(col_found) | ||
| error("谱系文件 '$filepath' 缺少必需列: $(first(aliases))") | ||
| end | ||
| rename_map[aliases[col_found]] = String(std_name) | ||
| end |
There was a problem hiding this comment.
Detect required pedigree columns using Symbols
The loop that verifies and renames the animal, sire, and dam columns searches names(pedigree) using raw strings (findfirst(name -> name in names(pedigree), aliases)). DataFrames names are Symbols by default, so this lookup never succeeds and the function always errors even when the CSV contains the expected headers. Converting aliases to Symbol before comparing (or converting names(pedigree) to strings) is required for load_pedigree to accept any real input.
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.
Split random-effect solutions before residual update
The REML M‑step computes residuals and trace terms by multiplying each Z matrix with solutions[n_fixed+1:end] and by using C_inv[current_pos+1:end, current_pos+1:end] for every random effect. When more than one random effect is defined, this uses the entire concatenated random-effect vector for every effect and mis-sized submatrices, causing dimension mismatches or incorrect variance updates. The random-effect blocks need to be sliced per effect (as is already done later for u) before multiplying with each Z and when forming the trace terms.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68de471a61a08321a08031b35c9b9e94