Skip to content

Feedback #1

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

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

Feedback #1

wants to merge 182 commits into from

Conversation

github-classroom[bot]
Copy link

@github-classroom github-classroom bot commented Sep 30, 2024

👋! 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:

  • Click the Files changed tab to see all of the changes pushed to the default branch since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
  • Click the Commits tab to see the commits pushed to the default branch. Click a commit to see specific changes.
  • If you turned on autograding, then click the Checks tab to see the results.
  • This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
    For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @taehan79-kim @chan-note @nOctaveLay @Two-Silver @Batwan01 @Donghwan127

github-classroom bot and others added 30 commits September 30, 2024 04:33
feat: YOLOv5 초기 구현 Issue #11 진행 중
train 부분 완료
Output metric.json to tensorboard form
nOctaveLay and others added 22 commits October 24, 2024 14:45
change pascal voc format to coco dataset
refactor: cahnge_pascal_to_coco 의 tqdm 개선
1. submission을 계속 접근하는 것이 아닌, 저장할 때만 접근하도록 변경
@KyubumShin
Copy link

Flie change 수가 너무 많아 line by line으로 리뷰를 남길 수 없어서 repo에서 코드 및 commit 로그, 이슈 확인 후 코멘트로 대신하였습니다.

가져온 오픈소스 코드들이 많아 실제로 작성한것 처럼 보이는 각각 모듈에 존재하는 inference 등의 코드를 참고해서 리뷰 드렸습니다

생각보다 여러가지 기능들을 직접 구현한 부분이 많고, 해당 코드들 대부분 가독성, 구조, 변수명, 코드에 관한 설명 등 다 잘 되어있어서 크게 문제되는 부분은 없어보입니다.

@KyubumShin
Copy link

KyubumShin commented Nov 3, 2024

다만 개인적인 생각으로 코드의 편의성 관련해서 이렇게 수정하면 좋을 것 같은 부분이 있는데요
해당 코드

해당 코드 부분에서 inference 시 config와 checkpoint path 및 output 파일명 까지 모두 직접 코드 단에서 수정해야할 필요가 있는데 이럴 경우 output 파일이 제대로 정리가 안될 경우 추후에 확인이 어려울 경우가 큽니다.
제 기억으로 mmdet의 경우 save path에 config도 저장 되는 걸로 알고 있는데 save path만 가지고 config, save checkpoint를 불러오고 output file 명까지 동일 경로명? or 동일 경로에 생성하도록 하면 관리가 더 깔끔해 지지 않을까 생각이 듭니다.

@KyubumShin
Copy link

그리고 tools/ensemble 경로에 csv파일이 존재하는데요 깜빡해서 체크하지 못하고 올라간것 같은데 삭제하는걸로 수정하시는게 좋을것 같습니다.

@KyubumShin
Copy link

마지막으로 csv 파일을 다룰때 팁 인데요
tools/ensemble.py 부분에서 score를 계산할 경우 일정 소수점 이하 자리를 자르는 형태는 제거하는 형식을 통해서 파일 크기를 많이 줄일수 있습니다. 혹시나 처리속도나 이런부분이 필요한 경우에는 위에 방법으로 시도하시는것도 좋을것 같습니다.

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.

6 participants