Skip to content

Conversation

@minad
Copy link
Member

@minad minad commented Oct 22, 2019

@czurnieden Regarding our discussion in #371, this is somehow what I would like to have. But the test is failing right now, maybe some rounding issue?.

@czurnieden
Copy link
Contributor

maybe some rounding issue?

Yes, but from an unexpected direction.
I was so concentrated on getting from log_2(x) to log_10(x) that I forgot to consider the rounding error in floor(log_2(x)) and now it's off by one and back where I started.

How preposterous of me to think that I'm better than the people at GMP! ;-)

There are some tricks available to get some more bits of information out of mp_count_bits but they all have quite some overhead:

e.g.: log(x^n)/n = log(x) (all numbers in decimal)
With one of the failures 1086654250330508058 and n = 2 we get floor(log_2(1086654250330508058)) = 119 round it up to 120 and divide by 2 to get 60 and feed that into the original machine to get the correctly rounded result. It is obvious that smaller numbers than that one need larger n and larger numbers get even larger when you square them.
Works quite well with floats but not with integers.

The other way is too take a couple of the most significant bits into consideration but I'm on my way already, needs to be put on hold until this evening.

@minad
Copy link
Member Author

minad commented Oct 23, 2019

No hurry! But I guess it is good to have it put into mp_log after all otherwise we wouldn't have caught this bug ;)

@minad
Copy link
Member Author

minad commented Oct 23, 2019

@czurnieden you can take over this PR if you want, it is your code after all.

@czurnieden
Copy link
Contributor

@czurnieden you can take over this PR if you want, it is your code after all.

In that case let me take over to show some mercy and put it down.
*sigh*

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