Closed lsc64 closed 1 month ago
Thanks for contributing to Opacus. Will take a look.
Thanks for contributing to Opacus. Will take a look.
Have you had the time to look into this? It seems like a straightfoward fix.
Thanks for contributing to Opacus! The fix makes sense to me. Just a qq: what is the point of using int(np,ceil())? How about just using math.ceil()?
Thanks for contributing to Opacus! The fix makes sense to me. Just a qq: what is the point of using int(np,ceil())? How about just using math.ceil()?
Thanks for pointing that out. Bad habit I guess, math.ceil is better. I changed it to math.ceil
This patch is still computing incorrectly the expected number of batches. See #516.
🐛 Bug
BatchSplittingSampler
reports the length asConverting the result simply to
int
leads to the resulted number of batches being one too low. Instead, we need to ceil the result first:Some libraries like pytorch lightning will skip the last batch if this is reported wrong, resulting in no actual step occuring at all.