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 #1

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

Feedback #1

wants to merge 185 commits into from

Conversation

github-classroom[bot]
Copy link

@github-classroom github-classroom bot commented Nov 10, 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: @SuyoungPark11 @kocanory @kaeh3403 @nOctaveLay @IronNote @kimmaru

github-classroom bot and others added 30 commits November 10, 2024 06:39
convert ipynb to python
baseline config file
🔀 Merge : 15-Streamlit 브랜치 병합
print char of each dice at wandb
designate wandb proj name
desinate wandb proj name
✨Feature: eda jupyter notebook 추가
📝 Docs write how to use the code
✨ Feature: EDA jupyter notebook 추가
nOctaveLay and others added 27 commits November 29, 2024 11:04
✨ 모델들을 불러오는 load_models 추가
✨ soft_voting을 하기 위한 soft_voting 함수 추가
🔥 test로 굴린 run_streamlit_test.py 코드 제거
classes, filename 구분하는데 _ 여러 개 있는 오류 방지하기 위해 수정
@@ -0,0 +1,100 @@
# Project Settings
proj_name : test
Copy link

Choose a reason for hiding this comment

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

config 부분을 매우 잘 정리해서 실험 한 것 같아보입니다.
체계적으로 실험한 면이 돋보이는데, 이런 config들도 실험 갯수가 많아지면 또 관리하기 어려우니,
주요 config들을 다시 표로 보기 좋게 정리해서 관리하는 것도 좋습니다.

@@ -0,0 +1,17 @@
## EDA 설명
Copy link

Choose a reason for hiding this comment

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

사용법을 매우 자세하게 정리한 점이 인상깊네요!
문서화가 매우 잘 되어있습니다.

@@ -0,0 +1,24 @@
## How to set up
Copy link

Choose a reason for hiding this comment

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

어떤 배경에서 이렇게 mmenv를 따로 디렉토리를 만들어 mmseg 환경으로 구분지었는지 충분히 이해가 갑니다.
다만 실제 프로젝트에서는 이런식으로 구분하는 것은 비효율적이고, 유사한 기능의 파일명, 함수명 등이 여럿 겹치게 되어 따로 모듈화 시킬 수 있는 방안을 살피는 것이 구조적으로 더 바람직합니다!

@@ -0,0 +1,17 @@
## EDA 설명

Copy link

Choose a reason for hiding this comment

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

데이터 전처리 코드는 따로 없는걸까요? 아니면 전처리 코드 없이 주어진 데이터를 그대로 활용하도록 한 것인가요?
해당 내용이 잘 명시되면 좋겠습니다.

┃ ┣ 📜config.yaml
┃ ┣ 📜soft_voting_config.yaml
┣ 📂data # 별도 설치 필요
┃ ┣ 📂test
Copy link

Choose a reason for hiding this comment

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

train 의 오타인가요?

Copy link
Contributor

Choose a reason for hiding this comment

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

오타라서 수정했습니다.

@@ -0,0 +1,47 @@
import os
Copy link

Choose a reason for hiding this comment

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

추론에 대한 기본적인 기능들은 잘 구현되어있지만, dice 이외의 다양한 metric으로 결과를 자세히 분석하는 내용이 추가되면 좋겠습니다. eda 코드를 따로 둔 것 처럼 결과를 상세분석하는 코드가 추가되면 좋겠습니다.

@@ -0,0 +1,587 @@
import datetime
Copy link

Choose a reason for hiding this comment

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

function.py 라는 이름은 너무 broad해보여요. 좀 더 모듈라이즈 한다면 유지보수에 더 유리할 것으로 생각됩니다!


filenames = []
labelnames = []
for i, (x, y) in enumerate(gkf.split(_filenames, ys, groups)):
Copy link

Choose a reason for hiding this comment

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

for문에 list를 +하는 것 보다 더 효율적으로 (더 짧은 코드로 더 빠르게 연산하도록) 구현하는 방법이 여럿 있습니다!
효율화를 위해 더 고민해보세요.

def __getitem__(self, item):

'''
그럼 train에서
Copy link

Choose a reason for hiding this comment

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

이건 무슨 내용일까요? ㅎㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

해당 내용은 삭제해도 될 듯 합니다.

## EDA 설명

1. streamlit을 활용하여 Train, Test, Inference한 데이터셋을 시각화하는 것을 목적으로 합니다.

Copy link

Choose a reason for hiding this comment

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

가급적이면 ipynb 이외에 .py 파일로 만들어서 특정 분석을 수행하면 그 결과들이 그래프라면 .png 등의 파일로, 수치나 표라면 csv 등의 파일로 떨어지도록 구현해보는 것도 좋습니다.

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