Skip to content

Improve data quality workflows - #17

Open
meibujun wants to merge 1 commit into
masterfrom
codex/implement-genomicprediction.jl-software-in-julia
Open

Improve data quality workflows#17
meibujun wants to merge 1 commit into
masterfrom
codex/implement-genomicprediction.jl-software-in-julia

Conversation

@meibujun

Copy link
Copy Markdown
Owner

Summary

  • add a QualityReport summary object and metadata aliases to the data pipeline for clearer downstream reuse
  • parallelize genotype standardization and quality control routines while enriching imputation with safeguards and telemetry
  • extend data processing tests to cover new metadata outputs and KNN fallback behaviour

Testing

  • not run (Julia toolchain unavailable in container)

https://chatgpt.com/codex/tasks/task_e_68f98c6e9d388321897d42279e9d067d

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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 "@codex address that feedback".

Comment on lines +386 to +401
function build_grm(dataset::GenomicDataset; method::Symbol = :vanraden)
X = dataset.genotype
n, p = size(X)
p == 0 && throw(ArgumentError("基因型矩阵为空, 无法构建 GRM"))
method == :vanraden || throw(ArgumentError("暂不支持的方法: $(method)"))

# 还原原始 0/1/2 计数以计算等位基因频率。
Z = copy(X)
denom = 0.0
@inbounds for j in 1:p
col = view(Z, :, j)
μ = mean(col)
col .-= μ
freq = (μ + 1.0) / 2 # 假设之前标准化后的均值约为 0
denom += 2 * freq * (1 - freq)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Compute allele frequencies from standardized columns incorrectly

The VanRaden GRM builder derives each locus frequency as freq = (μ + 1.0) / 2 after simply copying the genotype matrix. For raw 0/1/2 genotypes the correct estimator is mean(col) / 2; adding 1 forces frequencies ≥0.5 and makes common cases (e.g. μ ≈ 1 for p ≈ 0.5) yield freq = 1, driving 2*freq*(1−freq) to zero so the denominator collapses to eps(). When μ > 1 the term becomes negative. As a result the returned GRM is arbitrarily scaled or numerically unstable for any realistic dataset, breaking downstream models that expect a properly scaled relationship matrix.

Useful? React with 👍 / 👎.

Comment on lines +93 to +114
"""
build_transformer(seq_len, embed_dim, num_heads, num_layers; ff_dim=4*embed_dim,
dropout_rate=0.1)

构建轻量 Transformer, 利用多头注意力捕捉远距离位点关联。
"""
function build_transformer(seq_len::Integer, embed_dim::Integer, num_heads::Integer, num_layers::Integer;
ff_dim::Integer = 4 * embed_dim, dropout_rate::Real = 0.1)
encoder_blocks = Any[]
for _ in 1:num_layers
attn = MultiheadAttention(embed_dim, num_heads; dropout = dropout_rate)
ff = Chain(Dense(embed_dim, ff_dim, gelu), Dropout(dropout_rate), Dense(ff_dim, embed_dim))
push!(encoder_blocks, TransformerEncoder(attn, ff, LayerNorm(embed_dim)))
end
pos_enc = reshape(positionalencoding(embed_dim, seq_len), (embed_dim, seq_len, 1))
embedding = Dense(seq_len, embed_dim * seq_len, gelu)
return Chain(
x -> reshape(x, (seq_len, size(x, 2))),
embedding,
x -> reshape(x, (embed_dim, seq_len, size(x, 2))),
x -> x .+ pos_enc,
encoder_blocks...,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Positional encodings stay on CPU for GPU transformer models

build_transformer captures pos_enc inside the anonymous layer x -> x .+ pos_enc. When the model is moved to GPU via train_deep_model! (which calls fmap(cu, model)), that captured array is not transferred, so with device = :gpu the forward pass attempts to add a CPU matrix to a CuArray and raises a MethodError. This makes transformer training/inference fail on GPU despite the API exposing GPU support.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant