Support session inspection and fact-graph in ClojureScript
Closed this issue · 11 comments
Session inspection and the fact-graph functionality added in #277 are currently Clojure-only, but for the most part I don't think they need to be, although the :condition-matches are generated using Clojure-only functionality. In particular, I don't see any dependence of gen-fact->explanations on Clojure-only functionality, and this what the fact-graph functionality uses.
In addition to being useful for ClojureScript users, this enhancement would make it much easier to prototype visualizations using fact graphs in a browser or other JS-based environment since the problem of serializing a JVM fact graph, making it available via a web server, etc. could be put off.
@WilliamParker Not seeing how :condition-matches are CLJ-only. When you have time if you'd be able to elaborate this might be something I could help with.
@alex-dixon the key line is here. Basically it takes the productions (rules and queries) in the session, invokes a function in the Clara compiler on them, and then uses the result of that computation in the compiler to create the condition matches. Since the Clara compiler is Clojure-only a straight port of that to ClojureScript wouldn't work. Note that the Clojure-only compiler is invoked as part of macroexpansion in ClojureScript. Perhaps this could be implemented in a more portable way though; I just haven't had time to think about it yet. Ideally we wouldn't need to bring the compiler into session inspection anyway IMO. I'm certainly open to proposals on how to accomplish this.
Curious whether it's possible to use clara.listeners for this, in particular add-activations! and remove-activations!. If not, would it be possible to add listener code to surface the needed information in lieu of a solution that would involve the compiler?
alex-dixon@a79db09
master...alex-dixon:issue-307
Edit: Added fire-rule! event to listeners. This covers cases where facts are not inserted but the RHS still executes.
This probably isn't what we want. It requires adding a listener to the session to get explanations. Right now, given any session in CLJ, a user can inspect and get-explanations on it, which is more convenient and potentially more performant.
However, I think we can get the same information provided by the current explanation code in CLJS this way without needing to potentially rewrite part of the compiler to be more CLJS friendly.
Separate from this issue, it seems useful to include the token and node information in the add-facts and retract-facts listener methods. This appears to be the same data used to produce an explanation. It would be really useful for Precept because we attach a listener to every session anyway 😅 .
Further, with this approach we can detect what rules inserted facts unconditionally and the matches that caused the insert, something that appears unsupported currently according to http://www.clara-rules.org/docs/inspection/. We can also detect inserts and retracts from outside the rule context as well (currently just passing nil as the node and token arguments).
Ended up removing fire-rule! as it created too many events most of which were duplicates. For Precept we're thinking of strongly encouraging rules to insert facts always, including a "message" to accompany side effects if nothing else so the logic is trackable.
I've been able to get explanations in CLJS using listeners with the changes mentioned above. Let me know if this is of any interest and if not I can stop yammering about it.
It looks like we would like to go this route with Precept. I'd prefer not to depart from Clara with my own fork if at all possible. The diff should show the minimum changes to Clara to access the data for explanations via listeners. Passing node and tag arguments to a couple listeners that do not currently receive them seems like all that's required.
@alex-dixon Sorry I've taken so long to respond on this, I've been preoccupied lately with other work.
I've pushed a draft commit WilliamParker@c5863ab that I believe removes the dependency of the inspect namespace on the compiler. Basically the necessary information is contained within the beta-roots field on the Rulebase which is accessible through the components protocol function. This needs some more tests covering each node type with :condition-matches functionality, since the previous two paths (negation or join nodes in the intermediate compiler data structure) are turned into more paths since each of those two types corresponds to multiple node types in the engine, but the engine nodes in question have the necessary fields so I'm fairly confident the approach will work. Since the draft commit removes the compiler dependency I don't see any further barriers to fully transitioning the namespace to work in ClojureScript.
My preference would be to keep the base session inspection functionality independent of the need to have previously attached a listener. Since the tracing listener will maintain a reference to facts that otherwise would be garbage collectable I don't think attaching it by default is viable. However, I do see value in adding support in the tracing listener, if a user attaches one, to see where unconditional insertions and RHS retractions came from. I think there have been conversations in the Slack channel or maybe the mailing list about users wanting that functionality before as well. Perhaps that could be wired in to the session inspection functionality somehow so that if tracing was enabled it would pick up that data, but just adding it to tracing would be a worthwhile first step on its own. The changes in master...alex-dixon:issue-307 look like what I'd expect from doing that, although I anticipate we'd want to have some tests of the new functionality in test-tracing before merging them.
I've created a PR at #339 that refactors session inspection to not consume the compiler. This one uses the id-to-node field on the rulebase rather than the beta-roots, which won't actually contain everything necessary.
This doesn't add support for ClojureScript, but once the compiler dependency is removed it will more straightforward to add such support whenever myself or someone else has the time/interest to take it up again.
I believe I've got at least the inspect part of this working, just had to change the schema and inspect namespaces to be .cljc. Seems to work fine, tests pass, and am able to run explain-activations in the browser (which is proving to be super useful). I'll look at the fact-graph as well.
Are there any plans to do something like explain-activations for trace output? The trace shows the actual dynamics of the network, which in some cases gives you debugging information you won't get with explain-activations. Having a more readable representation would be nice.
@sparkofreason Regarding
Are there any plans to do something like explain-activations for trace output? The trace shows the actual dynamics of the network, which in some cases gives you debugging information you won't get with explain-activations. Having a more readable representation would be nice.
No concrete plans at this time, but something like that could be useful; if you have thoughts on how to create something like explain-activations for traces feel free to log an issue. I haven't thought of anything comprehensive for traces like explain-activations, though I have toyed with creating a standard library of functions for manipulating traces in a REPL for debugging. The thing I've always personally struggled with on traces is that for the real-world use cases I've dealt with the traces are so enormous that it wouldn't be practical to read any representation of them that didn't filter out a lot of data. Usually I've ended up doing a lot of ad-hoc processing in a REPL. Traces do contain a lot of useful debugging info that aren't in session inspection. That said, they also expose a lot of internals, so it isn't really safe to code against tracing output for production code unless you're prepared to track closely on the internals. That isn't a concern for manual debugging, diagnosis of performance problems, etc. though.
The anticipated use-case is debugging, specifically in the browser (thinking of a tool like re-frisk), maybe with some sort of query capability. I'll see where that goes, and if it looks like there's some functionality that would be useful to make commonly available to clara, I'll file a issue/PR.
I don't see anything left to do on this issue since #361 has been merged. Closing; we can reopen if there is disagreement.