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

Should we require the status list entries to be a multiple of the statuses per byte? #235

Open
TimoGlastra opened this issue May 19, 2024 · 5 comments

Comments

@TimoGlastra
Copy link
Contributor

e.g. if you have a status list with 4 status, and bit size 1, you encode this and then decode it, the decoded status list will have 8 entries, as you encode the status list per byte.

So I think we should maybe do a check that the <totalStatus> % (8 / <bitsPerStatus>) equals 0

E.g. a status list of length 256, with 1 bit per status = 256 % (8 / 1) = 0

However a status list of length 4 with 1 bit per status = 4 % (8 / 1) = 4

This will ensure the initial status list will always equal the decoded status list

@cre8
Copy link
Contributor

cre8 commented May 21, 2024

Mhh, I would not prefer to limit the user that he needs to think about what should be n so n % 8 = 0. I understand that the result of encoding and decoding in the future will not produce the same output for the first run.

Here we are calculating the size based on the byteArray, there is no hardcoded reference how many elements should be managed. And since we have to set a value for the bit in the byte, it is 0.

@TimoGlastra
Copy link
Contributor Author

As having implemented this functionality in Credo i spent quite a chunk of time debugging why my status list was longer than I configured it.

I thought it was a bug in the library, and finally i found that it goes per byte.

So passing 4 is the same as 8, i think it's useful to throw an error and say that you should pass a size of multiple of 8 / bitSize.

We can also implement this on the Credo layer, but i think unless there's a good reason why you would ever want to create a list of size 4 that ends up at 8 anyway, we should guide users to put in the correct length in the first place

@cre8
Copy link
Contributor

cre8 commented May 24, 2024

I would prefer a longer status list with more entries than intended, rather than forcing the use user to pick a specific value. Pick a random number has also no impact on the efficiency of the algorithm. When he says "I need a list of 799" elements, it will not break any use case when there are 800.

From the functionality aspect, a longer list will break no test or system. Because you do not now the final length because of the compression after this.

@TimoGlastra
Copy link
Contributor Author

Okay would like to hear some input of others on this (@lukasjhan, @berendsliedrecht). If everyone/most of us think it's ok to have that misalignment between input and actual created list I will close this issue.

@lukasjhan
Copy link
Member

If input a number that is not a multiple of 8, get a result that is different from the intended number of bits as @TimoGlastra mentioned. But I think it would be complicated to use if it throws an error.

I think it would be better to handle it gracefully rather than throwing an error.
So I'd like to suggest keeping the current direction and marking it clearly in the docs.

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

No branches or pull requests

3 participants