improve coverage

[sjaeckel]

I've noticed that the test-case for balanced multiplication wasn't hit anymore (or probably never? :-D ).

While looking at the coverage results I added some trivial ones and I think I found an inconsistency... nothing bad but it still stood out while looking at the code...

mp_prime_rabin_miller_trials:38 leads to 2 MR trials for numbers > 4096 bits

@czurnieden is that change fine that not only 4096bit sized numbers only have to do 1 MR but also numbers >4096bits?

moved to #269

[minad]

Is this is the inconsistency? I cannot judge this since I never looked into those prime tests. Ping @czurnieden?

[minad]

@sjaeckel Maybe split this PR in two if there is an issue with the prime tests, which needs more work?
balance_mul and the testing stuff looks good modulo nitpicks.

[sjaeckel]

split into two PR's as they're independent

[czurnieden]

mp_prime_rabin_miller_trials returns 39 in that special case which is too much for 8-bit. The test for t > MP_PRIME_SIZE fails because MP_PRIME_SIZE is only 31 for MP_8BIT.

There are many places available to repair/work-around that. Any preferences?

That test (under /* valid value of t? */) is obsolete, only random witnesses get used in mp_prime_is_prime now, the restrictions of ltm_prime_tab are irrelevant.

EDIT II:
There is a method available to run a sequence of M-R tests where the restrictions of ltm_prime_tab are relevant and that is if the value of -t is bigger than MP_PRIME_SIZE in the deterministic test. That was a failed attempt to keep some sort of downwards compatibility.
I think it should go, too, or do you want to keep it?

[czurnieden]

I've noticed that the test-case for balanced multiplication wasn't hit anymore (or probably never? :-D ).

I checked it with one of my (in)famous puts("DDD"); and it got hit, yes.

But it is better that way, of course.

[minad]

@sjaeckel Why is this failing? Maybe move all the prime sieve stuff to #269. Then balanced mul and fread/fwrite tests can already be merged.

[czurnieden]

Why is this failing?

I tried to explain it?

[minad]

@czurnieden Sorry, I didn't read your explanation first because I thought you commented here instead of in #269. I thought @sjaeckel moved all the "critical" stuff to #269.

[czurnieden]

Sorry, I didn't read your explanation first because I thought you commented here instead of in #269

I think I slowly grasp github's UI/UX but you're right, of course, to be still sceptical ;-)

But now that I have you: do you think it is enough to just delete the first test (could be done in this PR) or get rid of the second part, too (needs more work, including change of documentation, hence its own PR)?

[minad]

But now that I have you: do you think it is enough to just delete the first test (could be done in this PR) or get rid of the second part, too (needs more work, including change of documentation, hence its own PR)?

Hmm, I am not sure I really got you. Do you want to get rid of this ltm_primetab division test and replace it with your sieve? Then the specific ltm_primetab tests can go to (what you wrote under Edit II).

[sjaeckel]

I thought @sjaeckel moved all the "critical" stuff to #269.

Sorry, I thought so too :-) ... fixed now

[czurnieden]

Hmm, I am not sure I really got you.

Then let me assure you: you didn't—it seems as if I was not able to explain it with words.
So let me express myself in code.
Two PRs, one to repair the failure herein #269, one explaining what I meant.
(may take some time, am on an old laptop now that isn't the fastest)

[minad]

Then let me assure you: you didn't—it seems as if I was not able to explain it with words. So let me express myself in code. Two PRs, one to repair the failure herein #269, one explaining what I meant. (may take some time, am on an old laptop now that isn't the fastest)

Thanks! That's better. I'll add a comment in #272.