Skip to content

Update size of some persistent data types#962

Merged
gonzaponte merged 7 commits into
next-exp:masterfrom
gonzaponte:persistent-dtypes
Jun 8, 2026
Merged

Update size of some persistent data types#962
gonzaponte merged 7 commits into
next-exp:masterfrom
gonzaponte:persistent-dtypes

Conversation

@gonzaponte

Copy link
Copy Markdown
Collaborator

Due to the large number of reconstructed peaks, sometimes our counters surpass the maximum values for 8-bit unsigned integers. This PR promotes these counters to 16-bit uints, which should be enough in practice. Moreover, we add some overflow protection to avoid related issues that were silently going through before the update to python 3.13 (#955).

@gonzaponte gonzaponte mentioned this pull request Jun 4, 2026
@gonzaponte gonzaponte force-pushed the persistent-dtypes branch from 8dea80a to 3a0e1b8 Compare June 5, 2026 08:11
@jwaiton

jwaiton commented Jun 5, 2026

Copy link
Copy Markdown
Member

The overflow protection is nice, but shouldn't there be some warning that the user has reached the overflow limit? Having a bunch of peaks/sensor numbers with the same value may not be clear enough to the end user that they reached the int 16 bit limit.

@gonzaponte

Copy link
Copy Markdown
Collaborator Author

That's reasonable, yes.

@jwaiton jwaiton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR increases the memory allocation for peak and nsipm from UInt8Col to UInt16Col, increasing the maximal value from 255 to 65535!

Hopefully further increases wont be an issue for a while, and if it is, the user will be warned with the overflow checker also implemented in this PR.

Great work :)

@gonzaponte gonzaponte merged commit 2066fe5 into next-exp:master Jun 8, 2026
1 check passed
@gonzaponte gonzaponte deleted the persistent-dtypes branch June 8, 2026 14:21
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