Clojurians Log v2

That equality is crazy! Thanks!

https://dev.clojure.org/jira/browse/CLJ-867

http://clojuredocs.org/clojure.core/defrecord > and will defined Java .hashCode and .equals consistent with the contract for java.util.Map. Looks like this is intentional

(definterface Foo
  (getB []))

(deftype C [a]
  Foo
  (getB [this] a))

(deftype B [a]
  Foo
  (getB [this] a))

Clara just uses clojure =. Not sure the issue above? Clojure = takes type into consideration with records.

@mikerod The code that @thegeez referred to uses a java.util.HashMap, which uses java's notion of equality.

Fortunately for me, I am in an intersection of 2 points: 1) My data is all the same keys 2) I use macros for generating my types So I've been able to switch to beans with deftype rather trivially, and that's resolved everything.

@dominicm that’s interesting. I don’t know how you were encountering the use of platform/group-by-seq though. It does sound to me like there could be potential edge cases around platform/group-by-seq though on the jvm-impl since it does make use of Java collections to (a) preserve consistent order (b) do the grouping and order preservation efficiently.

@mikerod if the promise is that clojure = is used, than @dominicm’s example shows a bug

(defrecord ClauseOne [b])
(defrecord ClauseTwo [b])

(defrule rule-one
  [ClauseOne (= ?b b)]
  [ClauseTwo (= ?b b)]
 =>
  (println "Match found:" ?b )
  )

(defrecord WrapAlice [num])
(defrecord WrapBob [num])

(comment
  (clear-ns-productions!)
  (let [alice (->WrapAlice 1)
        bob (->WrapBob 1)]
    (println "(= alice bob)" (= alice bob))
    (println "(.equals alice bob)" (.equals alice bob))
    (-> (mk-session)
        (insert
         (->ClauseOne alice)
         ;; Line below is important, without it the wrong behavior does not surface
         (->ClauseTwo alice)
         (->ClauseTwo bob)
         )
        (fire-rules)
        ))
  ;;(= alice bob) false
  ;;(.equals alice bob) true
  ;; Match found: #clara_bug.core.WrapAlice{:num 1}
  ;; Match found: #clara_bug.core.WrapBob{:num 1} ;; <- should not happen
  )

@thegeez I think this actually is a bug

I had suspicion of it being one as soon as I saw this discussion above regarding platform/group-by-seq

platform/group-by-seq was added on the JVM-Clara to make the operation of grouping more efficient, as well as to have deterministic ordering of the seq it returns on each run to avoid variance in rule performance. The use of Java collections for this can cause subtle problems when the bindings of fact fields are record types that are Object.equals() from Java, but not clojure.core/=. This is the case with Clj record since their Java-interop form uses Java map equality instead of incorporating the type.

@mikerod this might be of use: https://github.com/ztellman/bizarro-collections to get a speedy group-by-seq with the semantics of clojure =

I believe the issue is fairly isolated to the platform/group-by-seq. I think the solution would be to stop using a Java-based equality hashmap impl. A clj based one would be needed instead. It may be slightly tricky to do to keep similar performance characteristics. (it’s a hot spot in the rule engine).

@thegeez maybe, but we need a “linked hash map”

the deterministic “order preservation” is an important characteristic

I believe the bizarro only has hash map (unordered)

a java.util.LinkedHashMap is used to do the grouping by operations, then it is seq’ed in platform/group-by-seq. The order the seq comes out, we want to be the same given the same input, independently from one process/machine to another (really, the Java runtime execution)

If we didn’t need the order preservation part, we could just use a clj transient map and likely not have a problem with performance

One solution would just be to write some sort of collection in Clara to fit this goal. That maybe the smoothest path forward.

https://github.com/amalloy/ordered supports transients.

I don't know of one of the shelve

But when does the case occur that the order is different between process? And when do these two different orderings meet?

@dominicm @thegeez I logged https://github.com/cerner/clara-rules/issues/393 - fixed link

it has quite a bit of detail on this

determinism from one JVM instance to another is pretty important in some production scenarios

platform/group-by-seq is hit in a hot spot in the engine - it has a fairly large effect on performance

the order of its results can play a large role in the activity that takes place within the engines evaluation of the network facts

If the order changes from run to run, then the performance characteristics change too. This can make it hard to track down a bottleneck that comes up only on occasion

Also, different orders of evaluation could potentially even lead to things like exceptions thrown in defects in written rules. However, may only sometimes these defects would manifest themselves - aka hard to reproduce problems

A while back, all known places within Clara’s engine evaluation and compiler were made deterministic run-over-run to alleviate these issues

This is quickly going beyond what I know of rules systems, which is very little. I'm only surprised to see the "order changes from run to run", I would expect the ordering to be arbitrary, but the same from run to run with the same input data

@thegeez the ordering guarantees are arbitrary from a user-level semantics

and the outputs are produced in a deterministic way from that level

I’m talking on more of a low-level evaluation within the engine

It is basically opaque to the user-level semantics

however, it has real world implications, (a) deterministic performance characteristics (b) deterministic order things actually are evaluated (mostly useful to reproduce error cases)

Things get more involved if you consider using the truth maintenance system within Clara, ie performing a typical clara.rules/insert! on a rule right-hand side (RHS).

The engine works to put the system into a consistent logical state

The “works” part, is where time is spent

There are different orders/paths to take to reach the same consistent logical state, depending on how rules are chosen for evaluation, how facts are propagated from node to node in the internal network etc

The determinism I’m talking about is in being deterministic in those inner choices taken

If "defsomething A, defsomething B, execute" should do the same as "defsomething B, defsomething A, execute" I would expect the usage of a HashMap instead of a LinkedHashMap, with its insertion-order preserving. That part is still fuzzy in my thinking

@thegeez I guess I’m not able to explain it correctly

Yes, they can do the same thing, however, the way they accomplish it may be different depending on the order

hash map can arbitrarily change the order

and it does

For example, we had correct rules running in a production case before. However, about 1 in 10 times, it performed about 100x worse

that 1 in 10 time, was when the hash map ordered 2 groups in a flipped way

the engine then went on to do extra work, it hit a performance bottleneck case

it still was functionally correct, it just went down a “bad path” performance-wise

fixing the determinism allowed the case to be pinpointed and recreated in a consistent way, the bottleneck was then fixed

There are many places within the engine evaluation where the “order things are done” can make measurable performance differences. Even if the outcome is always the same no matter the order.

It is hard to explain most of these, since they are detailed things about the engine/network/fact propagation

Also, truth maintenance can evaluate a rule, evaluated its RHS actions, like insert!, but then later, realize that that rule isn’t satisfied anymore due to some other rule that evaluated. The engine can “retract” the work done by the rule in that case.

There are cases (many) where it’d be better to never have evaluated that rule at all, it wasted time

The situation is really that the engine can at times “speculatively” evaluate a rule that is later found to be invalidated by other rules. There are ways for user-level directed rule orderings, and also there are engine-level “heuristics” or similar that attempt to avoid premature evaluation

So a rule could have a RHS that says (throw (Exception.))

This rule may never be true after fire-rules, however, during the truth maintenance cycle, it could actually be evaluated

So if you have non-determinism in that cycle, you may only sometimes hit the exception side-effect

It doesn’t (and likely wouldn’t) be something trivial like that. It could be something less obvious like (insert! (call-my-fn-that-has-a-defect-for-some-args ?x))

If that rule sometimes evaluated, but needs to retract later by the truth maintenance, and this only happens 1 in 10 times, you may have a 1 in 10 times sort of scenario you are trying to recreate to debug your defect in your rules

This will manifest itself in cases like, our tests fail arbitrarily at times when run on our build servers, but when we rerun the build/retest the error is gone

The determinism of ordering used within these areas of Clara help both of these scenarios, the performance situations, and the finding failures scenarios situation

@mikerod Thanks for the explanation

@mikerod ditto that was great

we should probably codify that mentality somwhere because we do think of it as first class here when considering changes

(here being cerner)

not sure where though. When writing rules it shouldnt be a consideration or a concern to the user

engine behavior? or guiding principles perhaps?

I think https://github.com/cerner/clara-rules/blob/master/CONTRIBUTING.md is intended for that purpose in the GitHub templates, perhaps something there?

that makes a degree of sense to me