Fork me on GitHub

That equality is crazy! Thanks!

dominicm05:05:14 > 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]
  (getB [this] a))

(deftype B [a]
  (getB [this] a))
Onto java beans I go!


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])

  (let [alice (->WrapAlice 1)
        bob (->WrapBob 1)]
    (println "(= alice bob)" (= alice bob))
    (println "(.equals alice bob)" (.equals alice bob))
    (-> (mk-session)
         (->ClauseOne alice)
         ;; Line below is important, without it the wrong behavior does not surface
         (->ClauseTwo alice)
         (->ClauseTwo bob)
  ;;(= 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: 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.


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?


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

👍 4

@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

👍 4

(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 is intended for that purpose in the GitHub templates, perhaps something there?

👍 4

that makes a degree of sense to me