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

Add binary sufixes to job tools memory specification (e.g. 1GiB vs 1GB) #2858

Merged
merged 1 commit into from
May 21, 2024

Conversation

h-mayorquin
Copy link
Collaborator

This is so primarly the ChunkRecordingExecturo can accept binary prefixes like 1 GiB, 500 MiB:

https://en.wikipedia.org/wiki/Binary_prefix

I think this is more useful for working with ram related things.

I am struggling with some memory errors while uploading data to the templates database and I am trying a more fine control so I thought this would be useful.

@h-mayorquin h-mayorquin added core Changes to core module concurrency Related to parallel processing performance Performance issues/improvements labels May 15, 2024
@h-mayorquin h-mayorquin self-assigned this May 15, 2024
@h-mayorquin h-mayorquin changed the title Add binary sufixes to job tools Add binary sufixes to job tools memory specification (e.g. 1GiB vs 1GB) May 16, 2024
@h-mayorquin h-mayorquin marked this pull request as ready for review May 16, 2024 01:24
Comment on lines +199 to +200
must end with "k", "M", "G", etc for decimal units and "ki", "Mi", "Gi", etc for
binary units. (e.g. "1k", "500M", "2G", "1ki", "500Mi", "2Gi")
Copy link
Member

Choose a reason for hiding this comment

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

@h-mayorquin I think that this might make things extra complicated. If I understand correctly,

1KB = 1000 bytes
1KiB = 1024 bytes

Is that correct? I had to look up the difference between decimal/binary for memory representation, so I think that an end user could be a bit puzzled by this.

Happy to discuss!

Copy link
Collaborator Author

@h-mayorquin h-mayorquin May 20, 2024

Choose a reason for hiding this comment

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

I think that having the two is confusing indeed.

But the current setup is confusing as well. If somebody allocates 1G using the current SpikeInterface machinery (and SpikeInterface was perfect in its handling of memory ...) the memory reported by the system won't match 1G as the user specified.

The reason for this is that RAM memory is reported in binary base. Both when you buy RAM and when you use utilities like system resource, the numbers you see are in binary base. .

Given that memory allocation here is a RAM issue, I think we should follow the convention of RAM specification (binary). However, we have to do things carefully because the decimal base has been there for so long.

Finally, given that using the job_kwargs is a relatively advanced feature, I am far less concerned about this type of confusion and I am fine with having the user specify which one they want to use.

(Hard drive storage, on the other hand, is reported in base 10, both when you calculate the size of an object in the system and when you buy a hard drive, here some link to the discssion on why this might be confusing for hard drives as well: https://superuser.com/questions/1080633/confusion-with-storage-capacity-powers-of-10-and-2)

@samuelgarcia samuelgarcia merged commit 6bb7567 into SpikeInterface:main May 21, 2024
11 checks passed
@h-mayorquin h-mayorquin deleted the binary_iec_prefixes branch May 21, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Related to parallel processing core Changes to core module performance Performance issues/improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants