Feedback #1
Open
Feedback #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-classroom[bot]](https://avatars.githubusercontent.com/in/68187?s=60&v=4){kind=small}
|
히히 신난다 |
Feat 5/issue and pr template
* 🎨 Add empty files and folders to make baseline structure(#4) * ✨ Setup config files for each model * ✨ Set up dataset folder contents * ✨ Set up train and trainer file * 🔥 Remove redundant inference file * 🚧 Add a temporary logger
* 🐛 Fix every bug * ➕ Add requirements file to show dependencies * 🚀 Add init files * 📝 Add how-to documentation
* ♻️ Refactor overall redundant codes * 🎨 Add config factory functionality * ✏️ Remove minor comments * 🐛 Fixed val_loss error and add error raising code when gpu is not detected * 🎨 Fix param_space to have more elegant representation * 🎨 Improve the pbt config * ♻️ Rename pytorch_lightning package as lightning * 📝 Update the precise guide * 🐛 Fix bug with moduel import * 🙈 Add .gitignore file * ✏️ Fix argparese argument to have dash * ♻️ Fixed the order of import * 🎨 Fix the old function of LightningModule * ♻️ Refactor the code and make loading checkpoint clearer * ♻️ Change the order of arguments for clarity * 🐛 Add a default model case * ✏️ Fix minor typo * ✏️ Fix typo * 🙈 Update to ignore lightning_logs * 🐛 Make dataset to read both train csv and test csv * 🚑 Make a quick fix for saving prediction csv file * 🎨 Add PredictionCallback * ✏️ Fix typo * ✏️ Fix typo * 🐛 Try to fix callback error * 🐛 Fix Expected a parent bug, see [the solution](Lightning-AI/pytorch-lightning#17485 (comment)) * 🐛 Move prediction feature from trainer to callback * 🙈 Update git not to read output csv file * ⬆️ Rewrite the requirements.yml
📝 Update documentation on train.py
* ♻️ Simple pylint refactoring * ♻️ Refactor to the higher level using class * ♻️ Refactor all the codes * 🎨 Make output csv file saved with the best checkpoint * ♻️ Stop train.py to make lightning_logs folder * 🎨 Simplify report callback * 🎨 Make a model_factory.py and lightning_module.py * 🎨 Updated RayTuner as a seperate file * ♻️ Add context manager to RayTuner class * 🔥 Remove redundant trainer.py * ♻️ Move test feature to test_runner.py and modify some function names to indicate their functionality * 🔥 Fix the locatoin of test_runner.py * 🚚 Rename documents for clarity * 🎨 Update on __init__.py files and organize imports * ♻️ Improve the imports order * 🐛 Add omitted return value of __enter__ * ♻️ Clarify that config passed to train_func is dict * 🎨 Update config structure * 🐛 Fix context manager bug * ♻️ Add nested dict structure * ⬆️ Add timm model * 📝 Update the documentations on tmux
Feat 23/hotfix data path
* 🐛 Moved from pbt to pb2 * ✏️ batch size typo
* ✨ Add custom checkpoint callback * 🐛 Enable logging and checkpointing(no duplicates?) * ✨ Change pbt, pb2 into ASHA and add the related logic * ✏️ Forgot to erase * ♻️ Change train_func as the static method of RayTuner * ✨ Add ability to disable ddp * ✏️ Fix typo * ♻️ Refactor some nested dict * ⚡️ Minor parameter adjustment
* ♻️ Update on nested config in each code for clarity * 🎨 Update on train transforms * 🔧 Update config structure on AHSA * ✏️ Fix typos * 🐛 Fix all the bugs related with lightning module hparams * ✨ Add LR scheduler * ✨ Add Albumentation transform * 🎨 Make it available to config everything in the config.py file * ⚡️ Make server to run concurrent trials * 🚑 Forget to add weight decay in the optimizer * ✨ Add a feature to run test from checkpoint_path * 🎨 Fix config to update properly from args * ⚡️ Change from Adam to AdamW * ✨ Add a feature to use pretrained model or not
* 💡 Add korean comments on config * 💡 Add korean comments on all files
conda env create 명령어에서 Python 버전은 환경 파일(requirements.yml) 내에서 설정되며, 명령어 자체에서 Python 버전을 지정하지 않아야 됩니다.
Update how_to_setup_and_train_kor.md
Update how_to_setup_and_train_kor.md
* ✨ ViT attn-only-fine-tuning * Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * ♻️ Change parameter print line to annotation * Add files via upload
Add uniform_soup.py
✨ val_loader update
* ✨ ViT attn-only-fine-tuning * Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * ♻️ Change parameter print line to annotation * Add files via upload * 🎉 Named ViT project * ⚡️ Chage val_dataset's split random seed * train 데이터셋으로 변경 * Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * Merge branch 'feat-70/att-only-fine-tuning' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * ✨ Add cross-Attention fine-tuning * Merge branch 'dataloader-modify' into feat-70/att-only-fine-tuning * ⚡️ Rename WandB project * 🐛 Fix annotation errors * 🔥 Remove code to model/ViT.py * ✨ Add DeiT3 freeze/unfreeze * Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * 🔀 Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feat-70/att-only-fine-tuning * ✏️ Fix typos
* ✨ Add amp ✨ Add amp * ✨ Add 16-bit mixed precision * Merge remote-tracking branch 'bcp1/main' into feature-90-1
Fixes #95
✏️ [BUG] Fix typo
* ✨ TrivialAugment 추가 ✨ TrivialAugment 추가 * Update ensemble.py 주석 부분들 다듬었습니다. * Update ensemble.py * Merge branch 'main' of https://github.com/boostcampaitech7/level1-imageclassification-cv-21 into feature-64
Devlee247
reviewed
Oct 7, 2024
|
|
||
| class ModelConfig: | ||
| """ | ||
| 이 클래스는 모델과 관련된 설정을 저장합니다. |
There was a problem hiding this comment.
3.8.4 참고하면 좋을 것 같습니다. 이 외 모든 docstring에서도 동일합니다.
| Args: | ||
| data_path (str): Path to the dataset. Defaults to '/home/data/'. | ||
| batch_size (int): Batch size for each data loader. Defaults to 32. | ||
| num_workers (int): Number of worker threads for each data loader. Defaults to 1. |
| Args: | ||
| data_path (str): Path to the dataset. Defaults to '/home/data/'. | ||
| batch_size (int): Batch size for the data loader. Defaults to 32. | ||
| num_workers (int): Number of worker threads for the data loader. Defaults to 1. |
| file_name = os.path.join( | ||
| self.ckpt_dir, f"{self.model_name}_predictions_{current_time}.csv" | ||
| ) | ||
| test_info.to_csv(file_name, index=False, lineterminator="\n") |
There was a problem hiding this comment.
올바르지 않은 file_name이 들어온 경우 handling하는 부분이 필요해 보입니다.
|
|
||
| # 체크포인트에서 모델 로드 | ||
| best_model = LightningModule.load_from_checkpoint( | ||
| f"{ckpt_dir}/checkpoint_DeiT3_ori0.ckpt", config=config.model |
There was a problem hiding this comment.
ckpt_dir이 올바르지 않은 경우 handling 해야하는 부분이 필요해 보입니다.
|
일단 전체적으로 잘 짜여져 있는 프로젝트 구조로 보입니다. 또한 pre-commit 및 issue-template을 잘 활용하였으며, 팀원끼리 정한 convention을 잘 활용해서 pull request 등 github를 제대로 사용해보자 라는 것이 느껴져서 좋았습니다. pre-commit tool이 있다 보니, 딱히 class나 function, module naming convention에 대해 얘기할 부분은 없어보입니다. 다만 최종 제출시 주석 처리된 불필요한 부분 / docstring에서 불필요한 부분 (https://github.com/Yosseulsin-JOB/Google-Python-Style-Guide-kor/blob/master/Google%20Python%20Style%20Guide/3.%20Python%20%EC%8A%A4%ED%83%80%EC%9D%BC%20%EA%B7%9C%EC%B9%99/3.8%20%EC%A3%BC%EC%84%9D%EA%B3%BC%20docstring.md) -> 구글은 신이니, 해당 링크 참고하시길 바랍니다. 그리고 너무나 자세하게 작성되어 있는 주석들은 오히려 가독성을 떨어뜨릴 수 있습니다. 이러한 부분을 감안하여 코드 리뷰를 진행했습니다. 마지막으로 하고 싶은 부분은 이 프로젝트에서 꼭 pytorch-lightning이 필요했는가? 입니다. 보통 pytorch-lightning이 pytorch에 비해 간결한 코드, 다중 GPU를 활용이 편함, checkpointing이 용이하다는 장점을 가지고 있지만 customizing 하기 어렵다는 단점이 있습니다. 본 프로젝트는 짧은 training 시간과 단일 GPU에서 진행이 되는 것으로 알고 있습니다. 따라서 이러한 점들을 고려 했을때 과연 최적의 선택이었나?에 대해 고민해보면 좋을 것 같습니다. (특히 디버깅하는데 많은 어려움이 있었을 것 같습니다.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
👋! 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:
For more information about this pull request, read “Leaving assignment feedback in GitHub”.
Subscribed: @boyamie @Haneol-Kijm @minseokheo @Tabianoo @suhyun6363 @kimmaru