Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<fix> integrate subset of retriever code & fill missing top-k parameter #26

Merged
merged 1 commit into from May 5, 2021

Conversation

SeongIkKim
Copy link
Member

  • 지영님 branch에서 추가된 retrieval argument
  • run.py에서 누락된 top-k 관련 파라미터 추가
    • config의 ST.json에서 retriever 추가 필요

종헌님이 이미 고치시고 commit만 남겨두고 있으신것 같지만 저도 돌려보느라 고쳐놓은 부분들 올렸습니다. 아직 지영님 브랜치 파일 정리가 다 안된것같아 merge해도 무리 없어 보이는 부분들만 제 브랜치에 적용시키고 종헌님 브랜치로 pull request 보냅니다!

BM 25 이슈는 아직 top-k개를 뽑아오지 못해서 정확도가 많이 낮은 문제도 있던것 같습니다.

Screen Shot 2021-05-05 at 7 25 50 AM

Screen Shot 2021-05-05 at 7 25 54 AM

위의 그래프는 기존 run 코드에서 건모님과 지영님이 구현해두신 top-k 샘플링 적용하여 query당 5개 지문 뽑아왔을 때의 EM입니다.
top-1 sampling에서는 BM25가 EM 6% 가량으로 더 낮게 나왔지만, 샘플링 수를 늘리자 13%가량으로 비슷해졌습니다. F-1은 오히려 근소하게 역전했습니다.

- 지영님 branch에서 추가된 retrieval argument
- run.py에서 누락된 top-k 관련 파라미터 추가
  - config의 ST.json에서 retriever 추가 필요
@ebbunnim
Copy link
Member

ebbunnim commented May 4, 2021

성익님, 혹시 bm25 돌리면 시간 어느정도 소요되셨나요??

@SeongIkKim
Copy link
Member Author

@ebbunnim koelectra 모델 기준으로 top-1 sampling사용할 때 ODQA 전체 14분 걸렸습니다! retriever 파트가 TF-IDF보다 더 오래걸리긴 합니다. TF-IDF는 전체 11분 30초 걸렸네요.

@olenmg
Copy link
Member

olenmg commented May 5, 2021

확인했습니다!

2 similar comments
@ggm1207
Copy link
Contributor

ggm1207 commented May 5, 2021

확인했습니다!

@ebbunnim
Copy link
Member

ebbunnim commented May 5, 2021

확인했습니다!

@olenmg olenmg merged commit 70c20a4 into jongheon May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants