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

Chunked C6 for Memory Efficiency #55

Merged
merged 8 commits into from
Apr 26, 2024
Merged

Chunked C6 for Memory Efficiency #55

merged 8 commits into from
Apr 26, 2024

Conversation

marvinfriede
Copy link
Member

No description provided.

test/test_model/test_c6.py Fixed Show fixed Hide fixed
test/test_model/test_c6.py Dismissed Show dismissed Hide dismissed
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Would it be better to provide an argument like memory and compute the chunk size based on the available memory?

@marvinfriede
Copy link
Member Author

Would it be better to provide an argument like memory and compute the chunk size based on the available memory?

I thought about it, but decided against it for several reasons.

  • First, it is not immediately clear, how much memory the calculations following the C6 build require (further usage of D3, gradients, ...). I played around with different chunk sizes and sometimes the calculation passed the C6 build, but still crashed later. So dynamically setting the chunk size might also cause memory issues later.
  • Second, if the available memory is used, the results might not be reproducible due to other processes taking up resources.
  • Third, I think, the memory is too crucial of a property to adjust automatically here. It should be the task of the user to properly set this up. With potentially non-reproducible behavior and OOM crashes at later stages, the problem might even become more difficult to diagnose for the user.

@marvinfriede marvinfriede merged commit 9dfd874 into main Apr 26, 2024
53 checks passed
@marvinfriede marvinfriede deleted the chunked branch April 26, 2024 07:41
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