Skip to content

Conversation

@czurnieden
Copy link
Contributor

@czurnieden czurnieden commented Oct 10, 2019

EDIT: The test-rig had a bug. Result after repair:
EDIT 2: The rigger himself had a bug. No repair possible.

This is the version with error range -0,+1
This version is exact for input x in the range 0 <= x <= 2^(2^31)) with bases b in the range 2 <= b <= 64 and has been fully(!) tested.

PR slicing as asked for in #343

@sjaeckel
Copy link
Member

how about calling this one mp_radix_size_overestimate_gmp(), the version of #368 mp_radix_size_overestimate_small(), then adding a mp_radix_size_overestimate() which branches depending on MP_LOW_MEM?

@minad minad added this to the v2.0.0 milestone Oct 14, 2019
@czurnieden
Copy link
Contributor Author

I have a faint but nagging idea that there is no overestimation here but a bug in the test code. See comment inside the code of #371
It will last about three days to check it fully. (one base is about half an hour on my machine and I can only use two cores at once or I'll loose the ability to watch…uhm…kitten videos in full 4k and I don't want to let it run unattended because the smoke-alarm is so loud it causes me headaches).

[...] which branches depending on MP_LOW_MEM

Ah, yes, I shouldn't have ignored that, thanks!

Mmh…is it (#386 ) even suitable for MP_LOW_MEM? It is itself larger, although not much, but it also causes the caller to reserve more memory, although not much in the smaller regions.

@czurnieden
Copy link
Contributor Author

I have a faint but nagging idea that there is no overestimation here but a bug in the test code.

Yes, three is a bug in the test code. Although both, the log and the division by the literal constant for log(2) are less than one ULP, both together can accumulate a slightly bigger error in a couple of cases (e.g.: 115 times for base 10, 92 times for base 7) for large input (>2^26) changing the result by one bit if it is too close to an integer which causes the final ceil to get irritated enough to allow for a wrong output.

Rerunning all(!) of the tests with binary96 (long double) to calculate the reference value resulted in zero errors, this table-based algorithm is exact for inputs 0 < x < 2^31.

The moral of this story? It is not always the code you test that's wrong, there are some rare, very rare cases when it it is the reference.

Outch!

That changes the landscape a bit, I think?

@minad minad changed the title Full version of mp_radix_size_overestimate mp_radix_size replacement O(1) for all bases, large tables Oct 22, 2019
@minad
Copy link
Member

minad commented Oct 22, 2019

@czurnieden ok to close this too?

@minad minad closed this Oct 22, 2019
@czurnieden
Copy link
Contributor Author

@czurnieden ok to close this too?

You already did ;-)

So you don't want this table-thing at all?

@minad
Copy link
Member

minad commented Oct 22, 2019

Well, who am I to decide. But I think #371 is more reasonable for most use cases.

@minad minad mentioned this pull request Oct 25, 2019
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.

3 participants