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

Update size of (continuous) benchmarks / Add benchmarks for new features #1661

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

mrfh92
Copy link
Collaborator

@mrfh92 mrfh92 commented Oct 4, 2024

Due Diligence

  • General:
  • Implementation:
    • unit tests: all split configurations tested
    • unit tests: multiple dtypes tested
    • benchmarks: created for new functionality
    • benchmarks: performance improved or maintained
    • documentation updated where needed

Description

Issue/s resolved: #

Changes proposed:

Type of change

Memory requirements

Performance

Does this change modify the behaviour of other functions? If so, which?

yes / no

@mrfh92
Copy link
Collaborator Author

mrfh92 commented Oct 7, 2024

@JuanPedroGHM do you think we should...

  • ...come up with some standardized sizes and shapes (as I started in this draft)...
  • ...or choose a size/shape individually for every function?

@mrfh92
Copy link
Collaborator Author

mrfh92 commented Oct 7, 2024

The first one would be quite easy, because you can choose a number of GB per process, resulting in a number of elements per process which then results in a total number of elements for the data set that then can result in a concrete shape (different for square, tall-skinny, very tall-skinny etc. data, but all with the same global and process-local amount of data)

@mrfh92
Copy link
Collaborator Author

mrfh92 commented Oct 8, 2024

looks like that it will become difficult to have a uniform size of the benchmarks...

@JuanPedroGHM
Copy link
Member

It's a good idea to align all our benchmarks by using standardized sizes, easier to design and change everything in the future if we get again more powerful hardware.

For the time being, I'm going to raise the time of the slurm job so that you can experiment better.

@JuanPedroGHM
Copy link
Member

Raised the time limit to 2 hours, but once you have more data fine tune it better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants