-
Notifications
You must be signed in to change notification settings - Fork 23
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
Time to match sets increases exponentially related to the set sizes #106
Comments
Thanks for this @pitalig. I agree that we should just be using set logic for sets. Are you interested in making a PR, or would you prefer that somebody else fix it? Either is fine. |
I will try to do it this week and then I share here what I got. |
@pitalig FYI, here's the same test using (require '[matcher-combinators.standalone :refer [match]])
(reduce
(fn [acc i]
(let [result (conj acc i)
expected (conj result (inc i))]
(println (time (match expected result)))
(println (str "set size: " (count result)))
result))
#{}
(range 10)) |
@sovelten and I explored something along these lines in the past. Not to say it shouldn't be looked into again, but wanted to share our observations: |
I don't know if looking at the predicates as equivalent classes will help because not always they will form a partition, for example: #{odd? even? multiple-of-3?} |
FWIW for our team I wrapped the While this is technically not correct (a match found after 17 minutes is still a valid test), we decided that if we hit this limit we split up the assertion or rewrite the test-case. The code can be found here: nedap/utils.test#28 |
I ran into this problem recently and ended up implementing the following, which worked alright for my use cases: (defrecord Sorted [expected]
matcher-combinators/Matcher
(-matcher-for [_this] (matcher-combinators/-matcher-for expected))
(-matcher-for [_this x] (matcher-combinators/-matcher-for expected x))
(-match [_this actual]
(let [sorted-expected (try (sort expected)
(catch Exception e e))
sorted-actual (try (sort actual)
(catch Exception e e))]
(cond
(instance? Exception sorted-expected)
{::result/type :mismatch
::result/value (model/->Mismatch (list 'sorted expected) actual)
::result/weight 1}
(instance? Exception sorted-actual)
{::result/type :mismatch
::result/value (model/->Mismatch (list 'sorted expected) actual)
::result/weight 1}
:else
(matcher-combinators.core/match sorted-expected sorted-actual))))
(-base-name [_this] (matcher-combinators/-base-name expected)))
(defn sorted
"Sorts both the expected and actual values before comparing them as sequences."
[expected]
(->Sorted expected)) Perhaps there are more elegant solutions (e.g. a strict set equality matcher), I'd be curious to hear other's thoughts. |
I believe that because set comparison tries to match all permutations, it is taking an impracticable time (with 10 items it took 17 minutes 😅) to test bigger sets.
Here is a test that I wrote to check that:
This was the result (just removed some noise like the facts results):
The text was updated successfully, but these errors were encountered: