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

Issues while updating to 0.10.2 #5

Open
sectore opened this issue Nov 17, 2016 · 9 comments
Open

Issues while updating to 0.10.2 #5

sectore opened this issue Nov 17, 2016 · 9 comments

Comments

@sectore
Copy link

sectore commented Nov 17, 2016

Hi Konstantin, to use purescript-strongcheck-generics with latest 0.10.2 I've updated few dependencies the package and added tiny changes to the code with the following commit: https://github.com/sectore/purescript-strongcheck-generics/commit/af7edf02e95ede30998c8d3df33f517f4995d954

psci is happy about all changes. However, it seems that the recursion of the Tree a example causes some issues while running the tests. The tests runs only if I disable all checks of Tree Int (see commit mentioned above).

Running the tests of props_gArbitrary without showSample / quickCheck using Tree Int

* Running tests...
100/100 test(s) passed.
1/1 test(s) passed.
[Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)),Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)),Test.Main.Nil,Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)))))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))]
* Tests OK.

Running the tests of props_gArbitrary with showSample / quickCheck by using Tree Int. Tests are "holded" as follows:

* Running tests...
100/100 test(s) passed.
1/1 test(s) passed.
[Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)),Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons Test.Main.Nil),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)),Test.Main.Nil,Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil)))))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))))),Test.Main.Cons (Test.Main.Cons (Test.Main.Cons Test.Main.Nil))]

It seems that the recursion of nested (Tree) data are failed. I dived into Test.StrongCheck.Generic.gArbitrary, also into Test.StrongCheck.Generic.genGenericSpine but I could not find out how to fix this issue.

Any idea?

Thanks!

-Jens

@zudov
Copy link
Owner

zudov commented Dec 14, 2016

Hi, thanks for help, and sorry for late reply.

I looked at it now (your points helped) and it appears that the problem is somewhere deeper, in the derived toSignature. I opened an issue here purescript/purescript#2482

@BartAdv
Copy link

BartAdv commented Jan 3, 2017

Since the purescript/purescript#2482 is closed, what's the status of this one?

@zudov
Copy link
Owner

zudov commented Jan 14, 2017

I gonna take another bite at this issue now.

@themoritz
Copy link
Contributor

@zudov I've had a look at the code and the issue is the SigArray case in genGenericSpine' (and in particular arrayOf).

This becomes problematic when combining Array with recursive data structures (like your Tree example), because for every Branch between 0 and 10 subtrees are generated, where 10 is the default size arrayOf uses. So the recursion only stops in the case of 0, and with very high probability you get an exponentially growing tree, so the code doesn't terminate.

There are two possible solutions:

  • Don't use arrayOf, but another distriubtion over array lengths, so that the tree grows with sub-exponential rate (for example [] and singleton with equal probability). However this makes the generated instances less interesting, and is not needed for non-recursive types.
  • Avoid the example in the tests, and warn about it in the documentation. Also, one could run the tree example with a size of 1 or 2. Compare here: https://github.com/purescript-contrib/purescript-strongcheck/blob/master/src/Test/StrongCheck.purs#L100

@themoritz
Copy link
Contributor

Hm, my reasoning can't be correct since it actually worked with PureScript 0.9.3. :) However, I was able to make it work with 0.10.5 generating empty or singleton arrays with 0.5 probability.

Another reason might be that strongcheck is just "painfully" slow since psc 0.10: purescript-deprecated/purescript-strongcheck#38

@garyb
Copy link
Contributor

garyb commented Jan 23, 2017

I wondered about that too. An easy-ish test would be to use quickcheck instead of strongcheck and see if it's still misbehaving. I'm not sure, but I don't think this uses any strongcheck specific features.

And by "painfully" I meant something like 10x slower, it shouldn't be bad enough to make things run indefinitely as seems to happen now.

@themoritz
Copy link
Contributor

Some observations after playing around with it:

  • If resizeing to 1, the Tree example takes 0.5s to complete.
  • If resizeing to 2, it takes 28.1s. The resulting Tree isn't really big, about 2/3 of a fullscreen terminal window.
  • If resizeing to 3, it takes > 30m (still running).

So it seems to me there is some exponential runtime bug somewhere, but I can't tell where...

@legrostdg
Copy link

Any news on this issue? It is preventing purescript-argonaut-generic-codecs, purescript-bridge, purescript-servant-support from upgrading to 0.10.x

eskimor/purescript-argonaut-generic-codecs#8
eskimor/purescript-bridge#16
eskimor/purescript-servant-support#4

@themoritz
Copy link
Contributor

I would be in favor of skipping the Tree test for now, so that the tests can pass and a release can be made.

And maybe put a warning somewhere about the combination of recursive data types and arrays.

zudov added a commit that referenced this issue Feb 5, 2017
zudov added a commit that referenced this issue Feb 5, 2017
A byproduct from the attempts to find what goes wrong there.
Should slightly help with further work on #5.
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

6 participants