⚡ Bolt: which.min()을 통한 O(N) 선형 탐색으로 p-value 최솟값 탐색 최적화 - #174
Conversation
R/surveyFA.R에서 p-value의 최솟값을 찾는 로직이 sort() 함수를 사용하여 O(N log N)의 시간 복잡도를 가지고 있었습니다. 이를 which.min()으로 대체하여 O(N) 선형 탐색으로 최적화하였습니다. 결과적으로 불필요한 정렬 오버헤드를 제거하여 속도를 향상시켰습니다.
|
👋 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. |
There was a problem hiding this comment.
Pull request overview
surveyFA()의 misfit item 선택 과정에서 최솟 p-value를 찾기 위해 전체 정렬을 수행하던 로직을 선형 탐색으로 대체해, 동일 목적을 더 낮은 시간 복잡도로 달성하려는 PR입니다.
Changes:
R/surveyFA.R:sort(...)[1L]기반 최솟값 선택을which.min()기반으로 변경하여 불필요한 O(N log N) 정렬 제거.jules/bolt.md: 관련 최적화 학습/액션 로그 항목 추가
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| R/surveyFA.R | p-value 최솟값 후보 선택 시 which.min()을 사용하도록 변경해 불필요한 정렬 비용을 제거 |
| .jules/bolt.md | 이번 최적화에 대한 학습/액션 로그를 문서에 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## 2024-06-25 - Replace O(N log N) sorting with O(N) linear scan | ||
| **Learning:** In R, finding the minimum or maximum element in a vector using `names(sort(x))[1]` incurs O(N log N) overhead due to sorting. | ||
| **Action:** Replace `sort()[1]` with `which.min()` (or `which.max()`) to achieve O(N) linear time complexity when finding extreme values in a vector. |
R CMD check 단계에서 발견된 NOTE(`.semgrepignore` 등 불필요한 은닉 파일 포함)를 해결하기 위해 `.Rbuildignore`에 정규식 예외 처리를 추가했습니다. 또한 이전의 `names(sort(p_values))[1]`에서 `names(which.min(p_values))`로의 교체 내용이 함께 포함되어 성능과 CI 안정성을 모두 확보했습니다.
| candidate <- names(which.min(p_values)) | ||
| if (!is.na(candidate) && p_values[[candidate]] < pThreshold) { |
| ## 2024-06-25 - Replace O(N log N) sorting with O(N) linear scan | ||
| **Learning:** In R, finding the minimum or maximum element in a vector using `names(sort(x))[1]` incurs O(N log N) overhead due to sorting. | ||
| **Action:** Replace `sort()[1]` with `which.min()` (or `which.max()`) to achieve O(N) linear time complexity when finding extreme values in a vector. |
R CMD check 단계에서 발견된 NOTE(비표준 파일 `test_dummy.R`, `test_validation.R` 등 포함)를 해결하기 위해 `.Rbuildignore`에 정규식 예외 처리를 추가했습니다. 또한 성능 향상을 위한 이전의 O(N log N) -> O(N) 최적화 내용이 함께 포함되어 성능 및 CI 안정성을 모두 확보했습니다.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
R/surveyFA.R:237
which.min()returns an unnamed integer index, sonames(which.min(p_values))isNULL. That makescandidatelength-0 and the subsequentif (!is.na(candidate) && ...)will error ("argument is of length zero") or skip the intended behavior. Use the index fromwhich.min()to look upnames(p_values)instead.
if (any(!is.na(p_values))) {
p_values[is.na(p_values)] <- 1
candidate <- names(which.min(p_values))
if (!is.na(candidate) && p_values[[candidate]] < pThreshold) {
return(candidate)
.Rbuildignore:29
- This newly added block is largely redundant with the existing anchored ignore rules above (e.g.,
.github,trivy.yaml,.yamllint.yml,AGENTS.md, etc.) and the broad^.*...$patterns are harder to reason about and can accidentally match more than intended. Consider keeping only a precise entry for.semgrepignorehere and relying on the existing anchored patterns for the rest.
^.*\.semgrepignore$
^.*\.yamllint\.yml$
^.*\.gitleaks\.toml$
^.*trivy\.yaml$
^.*CLAUDE\.md$
R CMD check 단계에서 발견된 NOTE(비표준 파일 `test_dummy.R`, `test_validation.R` 등 포함)를 해결하기 위해 `.Rbuildignore`에 정규식 예외 처리를 추가했습니다. 또한 성능 향상을 위한 이전의 O(N log N) -> O(N) 최적화 내용이 함께 포함되어 성능 및 CI 안정성을 모두 확보했습니다.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 55 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- aFIPC.Rcheck/00_pkg_src/aFIPC/man/autoFIPC.Rd: Generated file
Comments suppressed due to low confidence (1)
.Rbuildignore:35
.Rbuildignorealready has explicit entries for.github,.yamllint.yml,.gitleaks.toml,trivy.yaml, and the various docs. The additional^.*...$patterns here are redundant with the earlier anchored patterns and make the ignore list harder to audit (and easier to accidentally over-match future files). Consider deleting the redundant block and keeping the minimal, explicit patterns.
^.*\.semgrepignore$
^.*\.yamllint\.yml$
^.*\.gitleaks\.toml$
^.*trivy\.yaml$
^.*CLAUDE\.md$
^.*\.jules.*$
^.*\.Jules.*$
^.*AGENTS\.md$
^.*ARCHITECTURE\.md$
^.*CONTRIBUTING\.md$
^.*\.github.*$
|
Closing as superseded by #178, which now carries the same O(N) minimum-selection change with deterministic semantic-equivalence and domain-valid recovery tests in a bounded source diff. This branch also contains generated R check artifacts, so consolidating on #178 avoids shipping build output and competing exact-head checks. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
💡 무엇을 (What)
R/surveyFA.R에서 p-value의 최솟값을 가지는 항목을 찾을 때names(sort(p_values, decreasing = FALSE))[1L]대신names(which.min(p_values))를 사용하도록 변경했습니다.🎯 왜 (Why)
sort()를 사용하면 데이터 전체를 정렬해야 하므로 O(N log N)의 시간 복잡도가 발생합니다. 최솟값 하나만 찾으면 되므로, 선형 탐색(O(N))을 수행하는which.min()을 사용하는 것이 성능상 훨씬 효율적입니다.📊 영향 (Impact)
문제가 있는(Weird) 항목을 필터링하는 로직에서 불필요한 배열 정렬 과정이 제거되어
surveyFA함수의 실행 속도가 향상됩니다. 특히 제거 후보 아이템이 많을 때 성능 이점이 커집니다.🔬 측정 방법 (Measurement)
기존의 모든 유닛 테스트가 정상적으로 통과하는지 확인했습니다. 동작의 결과값은 이전과 완벽히 동일하며 불필요한 연산만 제거되었습니다.
PR created automatically by Jules for task 10601209340485051787 started by @seonghobae