[Open Source Contribution] YOLOv8(ultralytics/ultralytics) 기여 #3

https://github.com/ultralytics/ultralytics/pull/955

eval arguments with `iskeyword` and `locals()` instead of suppressing an exception for better readability by developer0hye · Pu

 

 

아래는 모델에 대한 config 파일이다.

 

# Ultralytics YOLO 🚀, GPL-3.0 license

# Parameters
nc: 80  # number of classes
depth_multiple: 1.00  # scales module repeats
width_multiple: 1.00  # scales convolution channels

# YOLOv8.0l backbone
backbone:
  # [from, repeats, module, args]
  - [-1, 1, Conv, [64, 3, 2]]  # 0-P1/2
  - [-1, 1, Conv, [128, 3, 2]]  # 1-P2/4
  - [-1, 3, C2f, [128, True]]
  - [-1, 1, Conv, [256, 3, 2]]  # 3-P3/8
  - [-1, 6, C2f, [256, True]]
  - [-1, 1, Conv, [512, 3, 2]]  # 5-P4/16
  - [-1, 6, C2f, [512, True]]
  - [-1, 1, Conv, [512, 3, 2]]  # 7-P5/32
  - [-1, 3, C2f, [512, True]]
  - [-1, 1, SPPF, [512, 5]]  # 9

# YOLOv8.0l head
head:
  - [-1, 1, nn.Upsample, [None, 2, 'nearest']]
  - [[-1, 6], 1, Concat, [1]]  # cat backbone P4
  - [-1, 3, C2f, [512]]  # 12

  - [-1, 1, nn.Upsample, [None, 2, 'nearest']]
  - [[-1, 4], 1, Concat, [1]]  # cat backbone P3
  - [-1, 3, C2f, [256]]  # 15 (P3/8-small)

  - [-1, 1, Conv, [256, 3, 2]]
  - [[-1, 12], 1, Concat, [1]]  # cat head P4
  - [-1, 3, C2f, [512]]  # 18 (P4/16-medium)

  - [-1, 1, Conv, [512, 3, 2]]
  - [[-1, 9], 1, Concat, [1]]  # cat head P5
  - [-1, 3, C2f, [512]]  # 21 (P5/32-large)

  - [[15, 18, 21], 1, Detect, [nc]]  # Detect(P3, P4, P5)

기존 코드를 보면 모델 parsing할때 args 라고 된 부분을 읽을때 string 값을 읽게되면 python 의 빌트인 함수인 eval 함수에 해당 string값이 입력된다.

 

이를 위한 기존 코드는 아래와 같았다.

for j, a in enumerate(args):
            with contextlib.suppress(NameError):
                args[j] = eval(a) if isinstance(a, str) else a  # eval strings

보면 NameError를 suppress 하는 부분이 있다.

 

이는 위 모델 config 파일에서 None은 eval로 잘 변환되지만 'nearest' 는 eval로 변환 불가한 값이여서 Exception 이 발생하는데 이때 발생하는 Exception 이 NameError다. 이럴때 트릭으로 발생하는 Exception을 무시하도록하여 구현이 돼있었다.

 

이게 너무 직관적이지 않고 이건 진짜 저 with문 지우고 실행해봤어야 왜 코드를 저렇게 작성했는지 해석이 가능한 부분이여서 이를 python 의 기본 제공 패키지 keyword 와 builtin 함수인 locals()를 이용해 구현했다.

 

리뷰어가 한명 붙어서 리뷰를 해주었는데 왜 keyword.iskeyword()를 썼냐 물어봤고 답을 해주었다. 다시 이 리뷰어는 그렇다면 보다 안전하게 ast.literal_eval을 쓰자는 거였다.

jocher도 이 리뷰어의 의견에 동의했고 테스트를 해보았다.

 

그치만...

 

이 리뷰어의 의견대로하니 Test 과정에서 모델 parsing과정에 오류가 발생했다고 한다.

 

결국에는 TODO로 넘어가게됐고, 구현은 원래대로 돌아왔다.

 

내 구현대로 하면 Test 과정 통과하는데... 아쉽다. 그치만, 뭐 eval() 은 취약하다고들 많이 말하니 eval을 제거하고 보다 안전한 ast.literal_eval 로 해당 과정이 구현될 수 있으면 따를만한 길인 거 같다. 시간나면 내가 다시 해보던가 해봐야겠다.