코드 리뷰 및 질의

olenmg commented 3 years ago

일단 제가 나중에 보려고 이해한 내용을 적어보았고, 아직 안보신 분들께도 도움이 될 수도 있어 올립니다 😊

run.py 동작

python -m run strategies ST01 --run_cnt 1

tool.get_args로 argument 가져오기
train_reader 실행
1. argument setup - tools.update_args 실행
  - update_args는 config에서 json 파일 불러와서 반영할(덮어쓸) argument를 args에 넣어주는 역할
2. dataset setup - prepare.prepare_dataset 실행
  - 지정된 경로에서 dataset 가져와서 반환, train/valid에 따라 구분
3. model & tokenizer setup - prepare.get_reader_model 실행
  - pretrained_model을 불러온다. (저장된거 or huggingface에서) ! (참고) args.model과 args.model_path
    - args.model = ModelArguments(model_name_or_path='monologg/koelectra-small-v3-discriminator', ...)
    - args.model_path = '/input/checkpoint/ST01_0_temp/checkpoint-500'
4. retriever setup - prepare.get_retriever 실행
  - tfidf retriever면 retrieval.SpraseRetrieval 객체 생성(이건 이전과 동일한 코드) ! 현재 다른 retriever에 대한 옵션 추가 필요, 물론 [다른 retriever]도 만들어야 함
5. retriever sparse embedding으로 세팅 ! 이것도 dense 코드 추가되면 분기 나눠줘야함
6. train dataset 전처리 - prepare.preprocess_dataset
  - 원래 베이스라인과 동일한 함수로 dataset 정제
7. valid dataset 전처리(1) - prepare.retrieve_pipeline, prepare.preprocess_dataset ! 여기서는 가장 관련성 높은 context를 찾아 반환한다. (ground-truth 말고 tfidf로 retrieve 예측된거)
  - 일단 retrieve_pipeline 실행
    - self.retrieve 실행
      - valid set의 전체 query에 대한 전체 문서의 유사도 구해서 가장 큰 거 score 및 index 반환
    - dataframe으로 넣기 위해 question, id, context_id, context, original_context, answers 정제 ! 여기서 id는 질문의 id(mrc-0-0000, ...), context_id는 지문의 id(0, 1, 2, ...)임 ! context는 예측한 지문, original_context는 ground truth 지문 ! answers 내에는 answer_start, text가 들어있음
    - dataframe으로 넣고 반환
  - dataset 형태 정하기 - predict 중인가? validation 중인가?
    - predict 중이면 answer는 모르니까 빼고 넣기
    - validation 중이면 answer도 넣기
  - dataframe 반환 ! 전처리 전 features: ['__index_level_0__', 'answers', 'context', 'document_id', 'id', 'question', 'title'] ! 전처리 후 features: ['answers', 'context', 'id', 'question']
    1. valid dataset 전처리(2) - prepare.preprocess_dataset ! retrieve한 문서가 valid dataset의 context로 설정된 상태
  - 원래 베이스라인과 동일한 함수로 dataset 정제
8. data_collator 불러오기
9. train, eval 모드 설정
10. train/evaluation

질문 및 제안

질문1

valid dataset 정제시 retrieve에서 실제로 활용하는 column은 위에서 언급했듯이 ['answers', 'context', 'id', 'question'] 뿐인데 다른 column값들(context_id, original_context 등)을 굳이 넣어야할까요? 아니면 나중에 다른 곳에 쓰일수도 있기 때문에 다른 부분도 넣는걸까요? (사실 이건 그렇게 주요한 사안은 아닙니다. 😕 가볍게 생각해주세요)

제안1

args.train.do_train, args.train.do_eval을 train_reader에서 설정해주지 말고, 사용자의 argument 입력으로부터 정보를 얻어와서update_args에서 값 설정해주고 아래

train_results = trainer.train()
eval_results = trainer.evaluate()

이 부분과 eval_dataset 불러오는 부분도 이에 맞춰 분기 나눠주면 좋을 것 같습니다.

제안2

추후 retriever가 여러개 생겼을 때를 대비하여 run.py 에서 retriever 불러오는 부분을 수정하는게 좋을 것 같습니다.

get_sparse_embedding()을 아예 get_retriever내부로 넣는 방향으로요!

수정 전

#run.py
...
retriever = get_retriever(args)
retriever.get_sparse_embedding()
...

#prepare.py 
...
def get_retriever(args):
    if args.model.retriever_name == "tfidf":
        from konlpy.tag import Mecab

        mecab = Mecab()
        retriever = SparseRetrieval(args, tokenize_fn=mecab.morphs)
    return retriever
...

수정 후

#run.py
...
retriever = get_retriever(args)
...

#prepare.py 
...
def get_retriever(args):
    if args.model.retriever_name == "tfidf":
        from konlpy.tag import Mecab

        mecab = Mecab()
        retriever = SparseRetrieval(args, tokenize_fn=mecab.morphs)
        retriever.get_sparse_embedding()
    elif ... 
    return retriever
...

리팩토링하려다가 실패한 부분

현재 post_processing_function을 train/eval dataset 전처리마다 한번씩 리턴해주고, train만 할때는 사용하지도 않는 함수라서 이 함수를 그냥 밖으로 빼려고 했는데 생각해보니까 args 변수때문에 밖으로 뺄 수가 없더라구요. 굳이 뺄 필요가 없긴 한데, 그래도 현재 코드가 완전히 깔끔한 구조라고는 볼 수 없을 것 같습니다. 이 부분에 대해서 아이디어 있으신 분은 제안 부탁드립니다!

SeongIkKim commented 3 years ago

post_processing_function 부분은 저도 읽으면서 불편하다고 느꼈습니다. 애초에 감싸고 있는 preprocess_dataset 파트에서 post_processing_function을 정의하고 만들어서 리턴해주는것도 좀 마음에 안들더라구요. Processor 클래스를 새로 만들어서 args, dataset, tokenizer등을 받아두고, preprocessing 파트와 postprocessing 파트를 분리하는건 어떨까요? 가능하다면 pre_XXX, post_XXX 등의 접두사를 붙여서 파트를 구분하고 처리 과정 자체는 최대한 메서드별로 잘게 쪼개고싶긴 합니다. Inner fuction도 그렇고 코드가 너무 길어서 읽기 괴로워요 ㅠㅠ

olenmg commented 3 years ago

써주신 의견 읽어보니 개선은 해야할 것 같은데 방법이 여러가지라 오늘 피어세션에서 관련 내용 짧게 논의해보는 것도 좋을 것 같습니다!

olenmg commented 3 years ago

리팩토링이 예정되어 더이상 도움 안될 것 같으니 이슈 닫겠습니다!

VumBleBot / odqa_baseline_code