Fork me on GitHub
#clojure-spec
<
2019-07-26
>
kenny16:07:52

Any good tools for debugging slow generators? We have lots of specs with tens of s/and preds & custom gens associated. I'd like to know which predicate is causing a such-that to fail often.

gfredericks16:07:14

I bet you could alter-var-root such-that to default to a lower max-tries

gfredericks16:07:27

looks like spec just uses the default (10) as far as I can tell

gfredericks16:07:43

so if you change that to, e.g., 2, you might get more failures

gfredericks16:07:14

but maybe that's not useful if the "telling you which s/and failed" feature isn't wired up yet

kenny16:07:27

> isn't wired up yet Is a feature like this on the roadmap?

kenny16:07:48

Good idea on the such-that max-tries thing. That should help a bit. Though, the such-that errors are usually pretty obtuse, hiding the actual name of the predicate behind an anonymously named test.check pred.

gfredericks16:07:00

spec's roadmap, yes, as far as I know; that feature couldn't exist prior to the 0.10.0 versions of test.check

gfredericks16:07:29

> the such-that errors are usually pretty obtuse that's exactly the problem that the aforementioned up-wiring would address

kenny16:07:58

Ah, very cool. Debugging these issues has been a huge time sink for us. It seems like it should be a fairly easy change to spec. What feature in 0.10.0 allows this better up-wiring?

gfredericks16:07:23

from the such-that docstring:

You can customize `such-that` by passing an optional third argument, which can
  either be an integer representing the maximum number of times test.check
  will try to generate a value matching the predicate, or a map:

      :max-tries  positive integer, the maximum number of tries (default 10)
      :ex-fn      a function of one arg that will be called if test.check cannot
                  generate a matching value; it will be passed a map with `:gen`,
                  `:pred`, and `:max-tries` and should return an exception

kenny16:07:35

I see. Would it be easy to hack this in to spec as a temporary fix?

gfredericks16:07:06

I have no idea, @U064X3EF3 probably knows at least the easy vs hard part

gfredericks16:07:58

I'd think you'd want have some kind of path on hand at the point where such-that is called

gfredericks16:07:12

so might take some wiring to make that path available

gfredericks16:07:32

or maybe it's already there, since spec likes to put paths in its error messages

kenny16:07:49

This would be a huge value add for us. If adding/hacking this in is a day's worth of work, it's definitely worth it for us to dive in

Alex Miller (Clojure team)16:07:57

it should be - gens have the path

Alex Miller (Clojure team)16:07:18

but they aren't using such-that right now

Alex Miller (Clojure team)16:07:31

so I'm not sure how this stuff connects

gfredericks16:07:43

they? this is about s/and in particular I think

gfredericks16:07:00

I can't imagine the gen for s/and doesn't use such-that

Alex Miller (Clojure team)16:07:57

I guess it does, just not directly

Alex Miller (Clojure team)16:07:10

that's generically in gensub, which handles the recursion check stuff

kenny16:07:44

It appears there's only one place that such-that is used in spec, gensub as you say. It also is passed other info that seems valuable to report back.

Alex Miller (Clojure team)16:07:01

are you using deps.edn ?

Alex Miller (Clojure team)16:07:12

if so I could commit something on a branch and you could test it as a git dep

kenny16:07:13

I currently don't have a repro of one of these such-that issues though, just the slow gen thing. The such-that errors are very frequent though

Alex Miller (Clojure team)16:07:27

easy to repro with any restrictive s/and pred

Alex Miller (Clojure team)16:07:42

my first hack did not yield anything useful though

Alex Miller (Clojure team)16:07:53

I need to step away for a bit, but may play with it later

kenny16:07:15

Ok. Let me know if I can help in any way.

Alex Miller (Clojure team)16:07:56

@kenny 95% of the time the issue with slow generators is generating large (and particularly nested large) collections

Alex Miller (Clojure team)16:07:38

setting :gen-max 3 in s/coll-of, s/map-of etc is often a big difference

👍 4
kenny16:07:59

This structure is highly nested & has lots of colls. We have these opts set:

:coll-check-limit 10
:coll-error-limit 10
:fspec-iterations 10
:recursion-limit  1

Alex Miller (Clojure team)16:07:49

none of those affect the gen'ed count

Alex Miller (Clojure team)16:07:04

with a 3 level coll, you could have 10x10x10 nested elements

kenny16:07:28

Oh, right - that param is on the actual generator. Lemme see if we have those set.

Alex Miller (Clojure team)16:07:55

well, recursion limit might affect the depth (although I think there are some scenarios where it is not getting applied right now)

Alex Miller (Clojure team)16:07:04

if you have fspecs, that's another possible issue where you might want to consider simpler preds

kenny16:07:03

We have considered that a number of times simply due to the massive amount of time debugging generators take. Every time, however, we always decide not to because those fspec generators catch valuable bugs haha

gfredericks16:07:33

the eternal test.check conundrum

✔️ 4
Alex Miller (Clojure team)16:07:40

another testing tip is that gens will get run 1000 times in check and size/complexity increases

Alex Miller (Clojure team)16:07:07

so gen/sample by default (20) will not expose generator issues

Alex Miller (Clojure team)16:07:17

but you can gen/sample with 1000 and will see those

Alex Miller (Clojure team)16:07:38

if you separate the args spec for an fdef into its own spec, it's easy to test that

Alex Miller (Clojure team)16:07:02

(s/fdef foo :args ::args-spec), then (gen/sample (s/gen ::args-spec) 1000)

kenny16:07:01

I searched for :gen-max in this service and didn't find any so it may not be used. Though, there are tons of custom gens which use the gen/* functions which don't take in :gen-max, I think. So that's the usual way to debug the slow gens -- something like this (do (doall (gen/sample (s/gen ::db/db-control-args) 1000)) nil). Then that will hang forever and that's where the pain starts.

Alex Miller (Clojure team)16:07:18

I would try just blindly adding :gen-max 3 to all the collection specs

gfredericks16:07:25

@kenny what version of test.check are you using?

Alex Miller (Clojure team)16:07:42

there is a ticket specifically for changing this default (or exposing a global default) btw

Alex Miller (Clojure team)16:07:45

which I am completely failing to find

Alex Miller (Clojure team)16:07:52

but I wrote the patch for it, so I know it exists

kenny16:07:17

The global default would still respect coll-of :min-count, right?

Alex Miller (Clojure team)17:07:18

there's actually a bug report about that

kenny16:07:24

Will try that. We're using 0.10.0-alpha3.

kenny16:07:42

Yeah, that would help to figure out if the problem is collection gen or something else.

gfredericks16:07:03

okay, that should avoid some exacerbatory issues with the 0.9.0 release

kenny16:07:32

Haha, yeah. I remember a huge perf bump when switching from 0.9.0

kenny16:07:09

Another thing that slows down debugging slow gens is that the check tests still run in the background even though I interrupted the current op in the REPL.

kenny16:07:34

I often will need to restart the REPL to get it to stop.

gfredericks16:07:44

is that a pmap thing?

kenny16:07:59

We're not using that. Does test.check?

gfredericks16:07:25

I think spec does

gfredericks16:07:30

but could be wrong

kenny16:07:23

pmap is in the check impl.

kenny16:07:21

Maybe a call to shutdown-agents would help?

kenny17:07:14

Getting a frequencies map returned after generating a spec with s/and would also be super helpful. The keys would be the forms of each s/and pred.

Alex Miller (Clojure team)17:07:21

well shutdown-agents will shutdown the pool never to restart again, so you'd still need to bounce your repl

Alex Miller (Clojure team)17:07:01

pmap is lazily parallel

Alex Miller (Clojure team)17:07:44

so when you stop using the result, up to n-1 (where n is number of processors) may still be working to compute the next few results in the lazy seq

kenny17:07:46

Hmm. It seems to go on forever. I’ll add prints to some functions to debug the slow gens and it never stops printing after interrupting.

kenny17:07:42

Perhaps because of the doall in this line? (do (doall (gen/sample (s/gen ::db/db-control-args) 100)) nil)

kenny17:07:10

This is helpful :

(let [such-that gen/such-that]
  (with-redefs [gen/such-that (fn
                                ([pred gen]
                                 (such-that pred gen 1))
                                ([pred gen opts]
                                 (such-that pred gen 1)))]
    (gen/generate (s/gen ::db-control-args))))

kenny17:07:59

Actually, @gfredericks the problem with taking the above approach is simple generators sometimes need the such-that. e.g.

(s/def ::int+-interval
  (s/and (s/tuple ::m/int+ ::m/int+)
         (fn [[x1 x2]] (>= x2 x1))))

kenny17:07:28

I suppose you could write a custom gen for an interval.

Alex Miller (Clojure team)19:07:07

there already is an interval spec with a generator - s/int-in

kenny19:07:36

That generates an integer. This one is an actual interval. e.g. [0 10]

gfredericks17:07:10

@kenny yeah the general rule with such-that is that the predicate needs to be highly-likely to succeed; so for something like an interval you'd want to write your own generator, which is pretty easy

kenny17:07:43

I mean, it is highly likely to succeed in the above case.

gfredericks17:07:42

isn't it just 50%?

kenny18:07:07

I think so

gfredericks18:07:12

more precisely what you want is "astronomically unlikely to fail 10 times in a row"

gfredericks18:07:24

and something that only passes 50% of the time doesn't have that quality

kenny18:07:38

Fair enough

gfredericks18:07:10

as such-that retries, it's also incrementing size, so often it's good enough that the probability of failure goes down with larger size

gfredericks18:07:14

e.g., gen/not-empty relies on that

kenny18:07:46

It almost seems like you should be able to enable a warning that tells you when your specs use s/and and don't have a custom gen.

kenny18:07:45

Writing the generator for an interval, like the above is tricky without such-that because you don't know how to generate a value that will match the spec. I was originally thinking it'd just be a gen/bind but that doesn't work because I don't know how to generate a ::m/int+ such that the second one is greater than the first without using such-that.

Alex Miller (Clojure team)19:07:14

the trick is to generate a starting value and the range size, then gen/fmap the pair from that

4
bfabry18:07:53

one option: before doing a such-that add something like this https://github.com/bfabry/specify-it/blob/master/src/specify_it/bst_spec.clj#L268 that just reports on how often the such-that would succeed. then tune your inputs until it’s a reasonable frequency

kenny18:07:59

Well this is a reasonable frequency but it's certainly not "astronomically unlikely to fail 10 times in a row"

kenny18:07:59

It's fairly unlikely though.

bfabry18:07:00

in the case that you’re going to throw away generations that don’t match your predicate, I would put “reasonable frequency” at like 50%

kenny18:07:30

That seems right. This would go back to the original problem then where, in practice, overriding such-that doesn't really work as a debugging technique.

bfabry18:07:13

a test.check generator mode where all of the various predicates print out their form line number and % success rate at the end would be cool (and quite hard to do)

kenny18:07:30

That's exactly what I want too 🙂

gfredericks18:07:10

@kenny w.r.t. avoiding such-that, how about fmap with sort?

kenny18:07:31

Ooo, I like it!

gfredericks18:07:46

fmap can be used to avoid such-that in a lot of cases another example is here: https://clojure.org/guides/test_check_beginner#_generator_combinators

Alex Miller (Clojure team)19:07:12

I would generate the starting value and the range size, then fmap to get [start (+ start size)]

kenny19:07:03

I don't think that'd work because you'd need to know how to generate the range.

kenny19:07:48

For example, we have a "prob-interval" where the interval's lower and upper bounds must be between 0 and 1.

kenny19:07:58

One issue with @gfredericks approach, however, is if you have a strict-interval. You have a chance of hitting the such-that then.

kenny19:07:04

Can't be that simple because of the above interval case.

kenny19:07:18

"prob-interval", that is

kenny19:07:02

You could add Double/MIN_VALUE in that case. But if the interval is integers only, that won't work.

kenny19:07:35

Well, even adding Double/MIN_VALUE may not work because that may go outside the range.

gfredericks20:07:02

Math/nextUp 🙂

kenny20:07:13

I think that's still problematic for the same reason though.

kenny20:07:03

Why isn't generator? public in spec's gen ns?

Alex Miller (Clojure team)20:07:39

Might be an oversight, might be a reason it won’t work with dynaload

kenny20:07:54

Same with vector-distinct-by

kenny20:07:31

Might be able to add them to our own libs by using lazy-combinators

kenny20:07:37

Could s/coll-of take a new option for :distinct-by?

souenzzo21:07:45

I need something like (s/tuple (s/tuple ::a) (s/tuple ::b)), but should allow values like [["a" "d"] ["b"]] (ignore "extra" tuple values)

souenzzo21:07:51

(s/cat :a (s/spec (s/cat :a ::a
                         :extra (s/* any?)))
       :b (s/spec (s/cat :b ::b
                         :extra (s/* any?))))
^ this one works, but it's really ugly

kenny21:07:39

Can you use a s/or?

kenny21:07:18

(s/def ::tup1 (s/tuple string?))
(s/def ::tup2 (s/tuple string? string?))
(s/def ::tup (s/or :tup1 ::tup1 :tup2 ::tup2))
(s/tuple (s/tuple ::tup ::tup))

souenzzo21:07:01

not sure how many ::tupN is needed. The fact is: just the 3 first values are required/used. (each one has it own spec) But sometimes a larger collection if passed as arg.

kenny21:07:07

Tuple is usually for a list of a known length. Sounds like you really want cat, like your example

👍 4