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.

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

Replace memory batch size with a fraction of the total batch size #359

Merged
merged 7 commits into from
Aug 17, 2023

Conversation

wistuba
Copy link
Contributor

@wistuba wistuba commented Aug 2, 2023

New attempt of #217

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Coverage report

The coverage rate went from 85.68% to 85.68% ⬇️

72.72% of new lines are covered.

Diff Coverage details (click to unfold)

src/renate/cli/parsing_functions.py

33.33% of new lines are covered (79.51% of the complete file).
Missing lines: 96, 111

src/renate/updaters/avalanche/learner.py

100% of new lines are covered (97.36% of the complete file).

src/renate/defaults.py

100% of new lines are covered (99.01% of the complete file).

src/renate/updaters/learner.py

83.33% of new lines are covered (96.8% of the complete file).
Missing lines: 459

Copy link
Contributor

@lballes lballes left a comment

Choose a reason for hiding this comment

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

Not sure why it works now, but LGTM.

In this version, offline ER still uses only batch_size in the initial update rather than batch_size + memory_batch_size, but I can adapt this in a separate PR.

@wistuba wistuba merged commit 73df5c2 into dev Aug 17, 2023
18 checks passed
@wistuba wistuba deleted the mw-redo-batch branch August 17, 2023 09:09
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.

2 participants