Skip to content

Fix gradient_accumulation_steps == 0 - #266

Open
Qubitium wants to merge 1 commit into
tloen:mainfrom
Qubitium:fix-zero-gradient-accumulation-math
Open

Qubitium wants to merge 1 commit into
tloen:mainfrom
Qubitium:fix-zero-gradient-accumulation-math

Conversation

@Qubitium

@Qubitium Qubitium commented Apr 4, 2023

Copy link
Copy Markdown

Due to float flooring math, the computed gradient_accumulation_steps value can be zero. Set min to 1.

Due to float flooring math, the computed gradient_accumulation_steps value can be zero. Set min to 1.
@Qubitium

Qubitium commented Apr 4, 2023

Copy link
Copy Markdown
Author

Example: Without fix, the following training config will generate zero gradient_accumulation_steps

torchrun nodes = 8
batch_size = 128
micro_batch_size = 32

@AngainorDev

Copy link
Copy Markdown
Contributor

Yep.
I'll propose a deeper check in a future PR, as all these params have to match and be consistent with each other and training dataset size.

For instance, in your case, you'll end up with a batch size of 32x8 = 256 and not 128.
A proper check would tell you need 256 and then no more rounding to 0.

This PR still can be useful as a safety in the mean time, but this more of hiding a misconfig.

@Qubitium

Qubitium commented Apr 6, 2023

Copy link
Copy Markdown
Author

What do you think of an alternative fix where:

  1. Output warning that config is likely wrong causing the zero value
  2. After warning, still let it continue by fixing gradient_accumulation_steps to 1, but also auto fixing the batch_size by doing doing node * micro_batch_size

@AngainorDev

Copy link
Copy Markdown
Contributor

What do you think of an alternative fix where:

Yep, what I had in mind was a consistency check of the entangled params, warning and auto fix if possible.
This would also take number of gpus into account and if run from python or torchrun.

A few sanity checks could avoid quite some issues there.

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.

4 participants