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

Overflow error in EuclideanRing #29

Open
jacereda opened this issue Sep 1, 2017 · 10 comments
Open

Overflow error in EuclideanRing #29

jacereda opened this issue Sep 1, 2017 · 10 comments

Comments

@jacereda
Copy link
Contributor

jacereda commented Sep 1, 2017

Checking 'Submultiplicative euclidean function' law for EuclideanRing
999/1000 test(s) passed.
/Users/jacereda/src/purescript/purescript-quickcheck-laws/output/Control.Monad.Eff.Exception/foreign.js:29
    throw e;
    ^

Error: Test 90 (seed 2114546612) failed: 
Test returned false
@hdgarrood
Copy link
Contributor

Which type is this? Also, how do you know it's an overflow error?

@jacereda
Copy link
Contributor Author

jacereda commented Sep 1, 2017

Int and I assume it's an overflow because the product can easily overflow with two arbitrary ints.

@jacereda
Copy link
Contributor Author

jacereda commented Sep 1, 2017

To reproduce it easily, just increase the number of iterations for submultiplicative in checkEuclideanRing, something like 10000 suffice.

@hdgarrood
Copy link
Contributor

Right, I guess we should just remove that test from this repo's test suite then.

@jacereda
Copy link
Contributor Author

jacereda commented Sep 1, 2017

Or perform an overflow check?

@hdgarrood
Copy link
Contributor

We can't do that with just a EuclideanRing constraint, though, surely?

@jacereda
Copy link
Contributor Author

jacereda commented Sep 1, 2017

nope, I guess Ord and Bounded would be required.

@hdgarrood
Copy link
Contributor

Right - and there are perfectly good euclidean rings which are neither Ord nor Bounded; gaussian integers and polynomials over a field, for example.

@jacereda
Copy link
Contributor Author

jacereda commented Sep 3, 2017

Looks like we can do without Bounded:

#30

@JordanMartinez
Copy link
Contributor

As discussed in #30 (comment)

Looks like the next action is to drop the overflow check and instead wrap Int in a newtype that limits how large the initial generated Int can be, so that an overflow error cannot occur. That would fix the false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants