refactor(glm): preserve binomial likelihood while removing ifelse - #111
refactor(glm): preserve binomial likelihood while removing ifelse#111seonghobae wants to merge 5 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough이항 GLM의 행렬 응답 정규화와 가중치 계산에서 Changes이항 GLM 계산 벡터화
Estimated code review effort: 2 (Simple) | ~10분 Merge Risk: ⚪ Minimal · up to This localized change replaces conditional vector construction with direct vectorized assignments in the binomial GLM likelihood path; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| y <- y[, 1]/n | ||
| y[n == 0] <- 0 | ||
| } else { | ||
| n <- rep.int(1, length(y)) | ||
| } | ||
| m <- if (any(n > 1)) n else wt | ||
| wt <- ifelse(m > 0, (wt/m), 0) | ||
| ## Bolt: replaced ifelse with preallocation and vectorized subsetting for performance | ||
| wt <- wt / m | ||
| wt[m <= 0] <- 0 |
There was a problem hiding this comment.
📝 Info: Zero denominators remain contained
Zero-trial and zero-weight rows create transient NaN values. The indexed assignments replace them before likelihood evaluation, preserving prior results.
Was this helpful? React with 👍 or 👎 to provide feedback.
| * checking for file ‘./DESCRIPTION’ ... ERROR | ||
| Required fields missing or empty: | ||
| ‘Author’ ‘Maintainer’ | ||
| * DONE | ||
| Status: 1 ERROR |
There was a problem hiding this comment.
🔍 Failed check artifact is committed
The generated 00check.log records a failed source check, not completion of the required tarball-based CRAN quality gate.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
seonghobae
changed the title
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
seonghobae
added
maintenance
priority: medium
ChatGPT Codex Connector
1 participant
범위
llcont.glm()의 binomial casewise likelihood 계산에서ifelse()를 직접 산술·부분 대입으로 바꾸되, 모델별 likelihood 의미와 zero-contribution 계약을 보존합니다. 이 변경을 아직 buyer-visible 성능 개선으로 승격하지 않습니다.이번 repair
cda75b392cb06637fe6d627ce073f2a53dc7daa9master@807e9405f8c32faafdf186f977a24d0b23358b43.jules/bolt.md의~22%repository-wide 성능 교리는 protected-base canonical blob으로 normal descendant 복원했습니다...Rcheck/00check.log는DESCRIPTION의Author/Maintainer누락으로Status: 1 ERROR인 로컬 실패 산출물이므로 branch에서 제거했습니다. 실패 로그를 package acceptance evidence로 보존하지 않습니다.tests/testthat/test_llcont_glm_binomial_zero_contract.R를 추가해 grouped-binomial의 zero-total row와 zero prior-weight row가 유한한 0 contribution을 유지하고sum(llcont(fit)) == as.numeric(logLik(fit))인지 직접 검증합니다.증거 경계
생성 PR의
1,000,000개 synthetic 벡터 microbenchmark와~208 ms -> ~144 ms,~22%주장은 제품 workload, fitted-GLM construction, runtime/CPU, warm-up/repetition, allocation/GC, failure denominator를 고정하지 않았습니다. 따라서 이 수치는 탐색 결과일 뿐 제품 성능 acceptance가 아닙니다.현재 source delta의 promotion 조건은 먼저 exact-head R package tests/R-CMD-check가 semantics를 검증하는 것입니다. 성능 개선으로 승격하려면 production-representative binomial model shapes에서 동일한 fitted-model/output 계약을 유지한 채 wall-clock/CPU/allocation evidence를 재현해야 합니다. 효과가 작거나 역행하면 더 단순한 구현으로 되돌립니다.
Current gate
Draft를 유지합니다. 현재 head의 hosted R-CMD-check/security/review evidence가 terminal GREEN이 되기 전에는 Ready/merge하지 않습니다. predecessor check, failed local Rcheck log, synthetic microbenchmark를 current-head GREEN으로 이전하지 않습니다. force-push, destructive rebase, self-approval, gate weakening은 사용하지 않습니다.