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.

Sign up for GitHub

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

Jump to bottom

Single-task POYO #25

Merged
merged 61 commits into from
Nov 29, 2024
Merged

Single-task POYO #25

merged 61 commits into from
Nov 29, 2024

Conversation

vinamarora8
Copy link
Member

@vinamarora8 vinamarora8 commented Nov 13, 2024 •
edited
Loading

Tested on entire MP dataset for up to 60 epochs. Works as expected. On 4xA40s: time per epoch: ~40s, time per validation: ~10s. I believe we are ready for training POYO-MP.

There is some ugliness wrt subtask_index handling. But I am deliberately leaving that until brainsets merges the interval-based approach so we don't have to re-engineer it later.

@vinamarora8
Changes BatchSampler to DistributedSampler for val/test dataloaders
1bc9c73 
Lightning doesn't play well with BatchSamplers in validation/testing
dataloaders
@vinamarora8
Fixes bugs in stitcher.py that show up in distributed setting
bf14232
@vinamarora8
Rename DistributedStitchingFixedWindowBatchSampler -> DistributedStit…
820ad78 
…chingFixedWindowSampler
@vinamarora8
Adapt test for Batch->Distributed sampler conversion
5896cc2
@vinamarora8
Basic funetuning and perf-base-modelcheckpoint setup
5d789f2
@vinamarora8
FreezeUnfreeze callback gets self.cl_mlog
53575bb
@vinamarora8
Support for testing
a4bf7da
@vinamarora8
Remove fast_dev_run from base.yaml
e1bda07
@vinamarora8
Comment
df887ec
@vinamarora8
Remove comment from ModelWeightStatsLogger()
6f910b8
@vinamarora8
Merge branch 'main' of github.com:neuro-galaxy/torch_brain into vinam…
6746ece 
…arora8-pro-159
@vinamarora8
Rename examples/poyo -> examples/poyo_plus
281b6da
@vinamarora8
Some initial models/poyo.py
16c50b7
@vinamarora8 vinamarora8 changed the base branch from main to vinamarora8-pro-159 November 13, 2024 21:27
@vinamarora8 vinamarora8 marked this pull request as draft November 13, 2024 23:34
@vinamarora8 vinamarora8 requested a review from mazabou November 16, 2024 02:45
@vinamarora8
Copy link
Member Author

@mazabou Ready for review. If possible, we should merge this in before adding examples/poyo/finetune.py

mazabou
mazabou reviewed Nov 18, 2024
View reviewed changes
Copy link
Member

@mazabou mazabou left a comment

Choose a reason for hiding this comment

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

Looks good overall. I left a few comments.

I made the update to brainsets, I can attempt to incorporate them here.

examples/poyo/README.md Show resolved Hide resolved
examples/poyo/configs/base.yaml Outdated Show resolved Hide resolved
torch_brain/utils/stitcher.py Outdated Show resolved Hide resolved
@vinamarora8 vinamarora8 requested a review from mazabou November 27, 2024 15:48
mazabou
mazabou reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@mazabou mazabou left a comment

Choose a reason for hiding this comment

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

Review of torch_brain/models

torch_brain/models/__init__.py Show resolved Hide resolved
torch_brain/models/poyo.py Outdated Show resolved Hide resolved
torch_brain/models/poyo.py Show resolved Hide resolved
torch_brain/models/poyo.py Outdated Show resolved Hide resolved
torch_brain/models/poyo.py Outdated Show resolved Hide resolved
torch_brain/models/poyo.py Outdated Show resolved Hide resolved
torch_brain/models/poyo.py Show resolved Hide resolved
mazabou
mazabou reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@mazabou mazabou left a comment

Choose a reason for hiding this comment

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

Review part 2 of 2

examples/poyo/train.py Outdated Show resolved Hide resolved
examples/poyo/train.py Outdated Show resolved Hide resolved
torch_brain/utils/callbacks.py Show resolved Hide resolved
torch_brain/utils/stitcher.py Outdated Show resolved Hide resolved
examples/poyo/configs/train_poyo_mp.yaml Outdated Show resolved Hide resolved
@vinamarora8
Copy link
Member Author

vinamarora8 commented Nov 28, 2024 •
edited
Loading

Note I have removed strategy=ddp_find_unused_parameters_true ... and accelerator = ... arguments of lightning trainer because we don't need them for POYO training.

@vinamarora8 vinamarora8 requested a review from mazabou November 28, 2024 07:51
mazabou
mazabou approved these changes Nov 28, 2024
View reviewed changes
Copy link
Member

@mazabou mazabou left a comment

Choose a reason for hiding this comment

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

Look great! Thank you for the colossal amount of work. Good to merge.

Just checking, you are going to merge this into main correct?

@vinamarora8 vinamarora8 changed the base branch from vinamarora8-pro-159 to main November 29, 2024 07:29
@vinamarora8 vinamarora8 changed the base branch from main to vinamarora8-pro-159 November 29, 2024 07:31
@vinamarora8
Copy link
Member Author

Look great! Thank you for the colossal amount of work. Good to merge.

Just checking, you are going to merge this into main correct?

Awesome. Glad we could finish this up. There is some weird entanglement between this and vinamarora8-pro-159. So it's just cleaner to get that branch into main before this. I will follow-up for that on #23

@mazabou mazabou changed the base branch from vinamarora8-pro-159 to main November 29, 2024 20:42
@mazabou mazabou merged commit 7a59bb5 into main Nov 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mazabou mazabou mazabou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vinamarora8 @mazabou