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

feedback pr2 #15

Open
wants to merge 72 commits into
base: feedback
Choose a base branch
from
Open

feedback pr2 #15

wants to merge 72 commits into from

Conversation

pica-git0
Copy link
Contributor

No description provided.

github-classroom bot and others added 30 commits November 8, 2024 09:54
프로젝트 폴더 구조 구성 및 베이스라인 구성
과하게 긴 evaluation 실행 시간으로 인해 evaluation이 최종 한 번만 실행되도록 변경
증강 데이터셋 실험 환경 구축
증강 데이터셋 실험 환경 구축 2차
feat: optimizer와 train lr을 같게 설정
현서님 지시대로 기존 optimizer 제거 후 SFTConfig의 optim 인자를 사용하여 adamw 8bit로 변경
qwen 데이터셋 실험 환경  optimizer 수정
한택님 SOTA 모델 파라미터 기반으로 config 파일 설정
…tput dir 변경

저장 간격을 최대한 좁혀 안정성 강화
is_augmented : 증강되었는 지 여부를 전달

데이터 증강 여부를 전달하여 기본 데이터셋와 증강 데이터셋의 차이를 고려하여 적절히 파싱하기 위해 추가
is_augmented 인자값(증강 데이터 여부)에 따라 적절히 처리하여 전처리된 데이터를 반환하는 파이프라인 구현
여러 output 병합하는 코드 추가가
다양한 로그를 저장하기 위한 폴더 세팅
--nohup 옵션을 통해 터미널 종료되어도 스크립트가 백그라운드에서 실행가능하도록 변경
@pica-git0 pica-git0 closed this Dec 9, 2024
@pica-git0 pica-git0 reopened this Dec 11, 2024
.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
config/.keep Outdated Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor 부분인데) yaml도 formatter 돌려주는 거 권장합니다 (prettier 같은 플러그인 추천)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

070b006 커밋으로 수정 제안 반영했습니다.



@dataclass
class Qwen32BWithUnsloth_ModelArguments(ModelArguments):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python class 명은 snakecase 사용하지 않는게 convention 입니다

Qwen32BWithUnsloth_ModelArguments -> Qwen32BWithUnslothModelArguments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6c29ded 커밋으로 문제 수정했습니다.

@@ -0,0 +1,53 @@
from ast import literal_eval
from typing import Dict, List, Optional
from xmlrpc.client import Boolean

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 안쓰인 거 같은데 남아있네요. requirements에도 없는거 같은데, 코드 돌리면 죽을 여지가 있네요

ruff 같은 툴은 다음 프로젝트에서는 꼭 사용해주세요

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a516092 커밋으로 수정했습니다.

Comment on lines +18 to +22
"question_plus": problems.get("question_plus", None),
}
# Include 'question_plus' if it exists
if "question_plus" in problems:
record["question_plus"] = problems["question_plus"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 중복이 있네요.
(이미 "question_plus": problems.get("question_plus", None) 로 처리됨)

return records


def prepare_records(dataset: pd.DataFrame, is_augmented: Optional[bool] = False) -> List[Dict]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional을 넣으면 안될꺼 같네요 (None이 들어오는 것은 의도된 것은 아닐것임)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

70c5037 커밋으로 수정했습니다.

return "".join(template)


def get_chat_template():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_default_chat_template 이라는 함수명으로 바꾸고,
다른 스크립트에 존재했던 chat_template은 다른 명칭으로 여기에 모두 추가해주면 어떨까 합니다

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

90da4a9
커밋으로 수정 제안 반영했습니다.

def __init__(self, model_name="beomi/gemma-ko-2b"):
self.model = AutoModelForCausalLM.from_pretrained(
model_name,
torch_dtype=torch.float16, ## 초기화 변수에 수정

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

float16도 __init__ 인자로 빼주세요

리뷰에 따라 custom rule을 맨 밑으로 옮겼습니다.
리뷰어의 수정제안을 반영했습니다.
리뷰어의 수정 제안에 따라 prettier 플러그인을 통한 포매팅 적용
리뷰에 따라 snakecase를 수정하여 PascalCase 적용
리뷰와 관련해 버그 픽스스
리뷰어의 제안 반영하여 오류 상태 종료로 변경
리뷰어의 제안을 적용하여 동적 너비 조정 적용용
리뷰에 따라 불필요한 주석 제거
리뷰어의 제안에 따라 함수명 수정
- 리뷰에 따라 심각한 문제를 유발할 수 있는 부분 제거
- python 자동 import 설정으로 인한 클래스 추가로 발생
)

리뷰어의 수정제안을 반영하여 메시지 템플릿 함수명 변경경
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.

2 participants