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

Improve indexation of the cummulated_loss_tensor in the Trainer class #123

Open
le1nux opened this issue May 3, 2024 · 0 comments · May be fixed by #122
Open

Improve indexation of the cummulated_loss_tensor in the Trainer class #123

le1nux opened this issue May 3, 2024 · 0 comments · May be fixed by #122
Assignees
@le1nux
Labels
enhancement New feature or request

Comments

@le1nux
Copy link
Member

le1nux commented May 3, 2024

As seen here,

modalities/src/modalities/trainer.py

Line 144 in a599005

post_processing_fun=lambda t: torch.stack([t[0] / t[-1], t[1] / dist.get_world_size()]),
, the cumulated_loss_and_gradient_norm is indexed directly in the train function, adding unnecessary complexity to the code.

A better solution would be to encapsulate the cumulated_loss_and_gradient_norm within a dedicated class that implements also the reduce operations.
@le1nux le1nux self-assigned this May 3, 2024
@le1nux le1nux added the enhancement New feature or request label May 3, 2024
@le1nux le1nux linked a pull request May 3, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@le1nux le1nux

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Parameter norm logging Modalities/modalities
1 participant
@le1nux