-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feedback #1
base: feedback
Are you sure you want to change the base?
Feedback #1
Conversation
Feat/baseline - 이슈 템플릿 업데이트 및 기타 작업
�FEAT: 베이스라인 코드 템플릿화
그거아세요? 제가 저번 플젝 때 푸바오를 철자를 틀렸다는 사실
Feat/ wandb 기능 추가 및 output.csv 파일명 변경
Feat/crawling
Feat/ensemble
fix: 공시 5지선다형 문제 추가
feat: extracting information from PDF documents, create txt files
feat:race_aug
feat: 외부데이터사용 시 대회 규칙 위배 여부 필터링
Feat/ 데이터 증강에 사용한 prompt
.github/ISSUE_TEMPLATE/config.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue Template을 통해서 프로젝트를 체계적으로 관리하고자 하시는 점이 좋았습니다. 실제 closed된 이슈들을 통해서 여러분들이 이러한 규칙을 잘 적용해서 진행하셨다는 점도 확인할 수 있어서 더 좋았던 것 같습니다.
.github/workflows/check-lint.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보통은 팀 프로젝트에서 github action을 통해서 lint까지 고려한 것도 좋았던 것 같습니다. 체계적으로 프로젝트를 관리하기 위해 다양한 시도했구나를 말씀드릴 수 있을 것 같습니다. 다만, 추가적으로 조금 더 고려한다면, 해당 부분에 대해서 특정 branch를 지정해서 trigger를 거는 것도 괜찮을 것 같습니다.
<img width="1000" alt="image" src="./assets/final_result.png"> | ||
|
||
## 🛠️**Dependencies** | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README에서는 해당 python이나 cuda 버전의 경우, dependency를 별도의 블록으로 표시해주시는 것이 어떨까 생각합니다. 실제 환경 설정을 할 때 조금 더 확실하게 볼 수 있지 않을까 싶습니다.
조금 더 나아간다면, script/setup-gpu-server.bash 관련 부분을 상위에 언급해주셔도 괜찮지 않을까 생각합니다.
tests/test_sample.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음과 같은 test성 코드의 경우 main branch 등에서 필터링하는 것도 괜찮을 것 같습니다. 프로젝트의 structure가 길어지면 그만큼 중요한 요소들에 대해서도 시선이 분산될 여지가 있습니다.
script/setup-gpu-server.bash
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음과 같이 공통된 환경 세팅을 위해 'bash' 파일을 통해 관리한 부분도 좋았습니다. 해당 부분의 경우 README에서 추가적으로 더 말씀하실 수도 있는 부분인 것 같다는 생각이 들어서 말씀드려봅니다.
|
||
|
||
if __name__ == "__main__": | ||
dataset_names = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
허깅페이스에 해당 데이터들을 올려서 사용하실 수 있게 해주신 부분은 좋습니다만, 혹 보안적인 부분이나 저작권적인 부분이 없을 지 염려가 되어 말씀드립니다.
data_process/crawling_gichulpass.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data_process 패키지 하위 모듈들을 보니, 전체적으로 너무 잘 정리해주신 것 같고 실제 랩업리포트에 작성된 부분들이 다 보이는 것 같았습니다. 다만, 배경지식이 없으신 분들에게는 해당 코드들의 역할이 궁금할 수 있을 것 같습니다. 그래서 해당 모듈들에 대한 주석이나 docstring이 작성되어 있으면 더 도움이 될 것 같다는 생각도 들었습니다.
tokenizer = self._load_tokenizer() | ||
return model, tokenizer | ||
|
||
def _load_model(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스 내부에서 사용되는 모델들에 대해서 함수명을 구분해서 작성해주신 점 좋은 것 같습니다!
from transformers import AutoModelForCausalLM, AutoTokenizer, BitsAndBytesConfig | ||
|
||
|
||
class ModelHandler: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 클래스에 대한 활용을 잘 하고 계신 것 같아서 코드가 되게 잘 정돈된 느낌을 받게 되는 것 같습니다.
config["inference"]["output_path"] = os.path.join(config["inference"]["output_path"], exp_name + "_output.csv") | ||
log_config(config) | ||
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음과 같이 각각의 역할군별로 클래스를 정의해서 사용하신 점 너무 좋은 것 같습니다.
멘토님 상세한 피드백 감사합니다 🙇 |
Main Branch 정리: 실험 및 작업 과정 커밋 제거
👋! 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: @yeseoLee @Sujinkim-625 @hsmin9809 @gayeon7877 @luckyvickyricky @koreannn