Feedback #1
Feedback #1
Conversation
[Style] Readme 초안 작성
[Fix] Update banner image
Develop refactor code
Develop refactor code
Feature/data process 데이터 프로세싱
[feat] 모델 ensemble
fix/readme 수정
IllgamhoDuck
left a comment
IllgamhoDuck
left a comment
There was a problem hiding this comment.
고생 많으셨습니다! train, inference의 폴더 구조, 그리고 실험을 config 단위로 나누는 것 까지 깔끔합니다. 코드 스타일을 통일하고 naming convention에 대한 합의를 이루어 가며 진행하면 더더욱 통일된 코드 스타일을 보장할 수 있을 것 같습니다.
commit message나 comment, docstring의 부재 혹은 설명이 자세히 쓰여있지 않고, hardcoding된 부분이 간혹 보이는 점을 개선하면 더욱 더 유연하고 확장성 있는 코드가 될 것 같습니다. 랩업 리포트에 읽은 내용들이 코드로 어떻게 녹여들어갔는지 읽는 즐거움이 있었습니다.
여기서 더 나아간다면 협업을 위한 더 자세한 주석 및 exception handling 정도가 추가되면 좋을 것 같습니다. 전반적으로 정석적인 인공지능 실험 폴더 구조라 보기가 편했고, 팀원간의 코딩 스타일만 다음부터 잘 상의하셔서 통일시키는 것만 하면 좋을 것 같습니다.
|
|
||
|
|
||
| # RD 제거 버전 | ||
| def EDA(sentence, alpha_sr=0.1, alpha_ri=0.1, alpha_rs=0.1, num_aug=9): |
There was a problem hiding this comment.
자잘하지만 pep8에서는 함수명에 대문자를 사용하지 않습니다!
| @@ -0,0 +1,188 @@ | |||
| # 데이터 증강 | |||
There was a problem hiding this comment.
파일명 자체가 eda가 아니라 eda의 세부 카테고리면 더 좋을 것 같습니다. eda/hangul.py?
There was a problem hiding this comment.
config.yaml보다는 baseline이나 다른 이름이 더 좋지 않을까 합니다
| with open("./config/config.yaml", encoding="utf-8") as f: | ||
| CFG = yaml.load(f, Loader=yaml.FullLoader) |
There was a problem hiding this comment.
inference는 python 인자를 받을 수 있게 하는 것이 좋아보입니다. args를 사용하지 않고 hardcode하는 것은 지양하면 좋을 것 같습니다. 새로운 config 실험이 어려워질 것 같다
| CFG = yaml.load(f, Loader=yaml.FullLoader) | ||
|
|
||
| # 저장된 폴더 이름 가장 최근걸로 불러오기 | ||
| exp_name = get_latest_experiment_folder() |
There was a problem hiding this comment.
가장 최신 실험 폴더도 좋지만 이렇게 코드를 짤 경우 2명이 이상이 실험을 하면 마지막 실험 폴더의 결과만 출력하게 됩니다. 마지막 폴더 자동 지정 방식보다 직접 실험 폴더명을 지정할 수 있게 해주는 것이 좋을 것 같습니다.
|
|
||
| for word in words: | ||
| check_result = CheckResult.PASSED | ||
| if word[:5] == "<red>": |
There was a problem hiding this comment.
word에 어떠한 예시가 들어오는지 위에 주석을 달아주면 좋을 것 같습니다. 코드의 흐름을 따라가기가 어렵습니다. 특히 [:5]같은 경우는 아는 사람만 알고 모르는 사람은 모를 확률이다
|
|
||
| def check(text): | ||
| """ | ||
| 매개변수로 입력받은 한글 문장의 맞춤법을 체크합니다. |
There was a problem hiding this comment.
check라는 함수를 더 명확한 의미를 담기게 표현하면 좋을 것 같고 단계별로 docstring에 절차를 써주면 더 좋을 것 요
| BERT_aug = BERT_Augmentation() | ||
| random_masking_replacement = BERT_aug.random_masking_replacement | ||
| random_masking_insertion = BERT_aug.random_masking_insertion | ||
| adverb_aug = AdverbAugmentation() | ||
| adverb_gloss_replacement = adverb_aug.adverb_gloss_replacement | ||
|
|
||
| orig_train = pd.read_json("sts/datasets/klue-sts-v1.1_train.json") |
There was a problem hiding this comment.
함수가 아닌 일반 file에 바로 실행 함수를 두는 것은 권장되지 않습니다. K-TACC augmentation이라는 것은 알겠지만 K-TACC의 증강 함수를 가져와서 적용하는 형태여야지, K-TACC의 augmentation일부분만 적용하고 싶은 상황에서도 이런 형태라면 모든 증강이 실행되게 됩니다.
각 함수를 선별적으로 사용하게 하고 전체를 한번에 실행시켜야 한다면 그런 실행 함수를 별도로 만드는 것이 더 나을 것 같습니다.
| try: | ||
| for syn in wordnet[word]: | ||
| for s in syn: | ||
| synomyms.append(s) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
이건 띄어쓰기가 다르게 설정된 것으로 보여집니다. 아마 처음에 협업하실때 2칸 4칸 등의 tab크기도 서로 상의해서 통일시키는 것이 좋을 것 같습니다!
| torch | ||
| transformers | ||
| kiwipiepy |
There was a problem hiding this comment.
케바케이긴 하지만, 해당 라이브러리를 동작시키는 시점의 버젼을 기록해주면 좋습니다. 아니면 dependency가 업데이트되면서 backward compatibility가 보장이 되지 않는다면 문제가 발생할 확률이 있습니다.
👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.
Notes for teachers
Use this PR to leave feedback. Here are some tips:
For more information about this pull request, read “Leaving assignment feedback in GitHub”.
Subscribed: @cukminseo @Sujinkim-625 @yeseoLee @hsmin9809 @koreannn @gayeon7877