perf(ipd): evaluate explicit factor levels with equivalence coverage - #323
perf(ipd): evaluate explicit factor levels with equivalence coverage#323seonghobae wants to merge 2 commits into
Conversation
Replaced `as.factor()` with `factor(..., levels=c('newForm', 'oldForm'))` to avoid O(N) overhead associated with level inference and string sorting during the factor creation.
|
👋 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. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
| ## 2026-09-02 - R 언어에서 factor 생성 시 명시적 수준 지정으로 O(N) 오버헤드 최적화 | ||
| **Learning:** R에서 `as.factor()`를 사용해 요인을 생성하면 데이터를 스캔하고 정렬하여 레벨을 추론하는 오버헤드가 발생합니다. | ||
| **Action:** 레벨이 이미 알려진 경우 `factor(..., levels = c(...))` 형식으로 레벨을 명시적으로 제공하여 추론 과정을 우회하고 성능을 최적화할 수 있습니다. |
| factor(c( | ||
| rep('oldForm', nrow(oldformYDataK)), | ||
| rep('newForm', nrow(newformXDataK)) | ||
| )) | ||
| ), levels = c('newForm', 'oldForm')) |
There was a problem hiding this comment.
Current exact authority
master@f87c2324f1686135e57d8730c1b0b9420874f300249217b00d92802ab4bccaafcb02e00fe093cb84R/aFIPC.R,tests/testthat/test-ipdgroup-factor-equivalence.RReview and repair
The production delta replaces
as.factor(c(rep("oldForm", ...), rep("newForm", ...)))withfactor(..., levels = c("newForm", "oldForm"))in the IPD grouping path.The generated PR originally claimed roughly 30–50% factor-construction improvement without a reproducible representative benchmark or profile. Unit success cannot establish that number or a buyer-visible
autoFIPC()speedup, so it is not merge authority for this lane.A normal descendant restores
.jules/bolt.mdbyte-for-byte to protected authority (7d3c603f9991196ce123271a6c1e129156f24fd8). This local expression experiment is not repository-wide performance doctrine. No force push or destructive rebase was used.The new deterministic characterization checks the exact protected factor object against the explicit-level candidate for several non-empty old/new form cardinalities, including factor levels and internal encoding through
expect_identical(). Synthetic cardinalities here are unit-test fixtures only; they are not psychometric or performance acceptance data.Remaining RED / GREEN acceptance
The characterization currently covers the product path where both forms contribute observations. Before claiming behavior equivalence for the whole public input domain, either prove the maintained
autoFIPC()path rejects/never reaches IPD with an empty form before this expression, or characterize and preserve the protected one-sided/empty-factor semantics explicitly. Do not silently redefine that edge case to make the optimization pass.Performance promotion additionally requires a protected-vs-candidate benchmark on the actual IPD path using representative/right-cleared form data under the same R/mirt/runtime state, with respondent/item cardinalities, warm-up policy, repeated wall-time distribution (at least median and p95), allocation/GC evidence, and profile evidence showing this constructor is material. Do not extrapolate a micro-expression timing to calibration latency.
Keep Draft until one unchanged exact head has terminal R CMD/test, applicable security/SAST/CodeQL, current review findings resolved, and qualifying independent review. No self-approval, source-neutral retrigger, gate weakening, generated Bolt doctrine, force push, or destructive rebase.