-
Notifications
You must be signed in to change notification settings - Fork 115
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
Support session inspection and fact-graph in ClojureScript #307
Comments
@WilliamParker Not seeing how |
@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 |
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 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 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 |
Ended up removing 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 |
@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 Are there any plans to do something like |
…fact-graph namespaces to CLJC. Provides support for session inspection and fact-graph in ClojureScript (oracle-samples#307).
@sparkofreason Regarding
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. |
…#361) * Convert the clara.rules.schema, clara.tools.inspect, and clara.tools.fact-graph namespaces to CLJC. Provides support for session inspection and fact-graph in ClojureScript (#307). * Update def-rules-test to include the rule/query name in the productions, make rule/query bindings available in generated let-block. Port clara.tools.test-fact-graph and clara.tools.test-inspect tests to common using def-rules-test. * Remove redundant bindings for def-test-rules. Correct CHANGELOG.md and CONTRIBUTORS.md. * Add new CLJC tests to src/test/clojurescript/clara/test.cljs so CLJS versions run. Fix errors in CLJS tests. * :require goog.string instead of :import, use alias. * Have clara.tools.testing-utils :require-macros on itself to support implicit macro inference. Use explicit namespace alias instead of :refer/:refer-macros when using clara.tools.testing-utils. Clean up some namespace definitions.
I don't see anything left to do on this issue since #361 has been merged. Closing; we can reopen if there is disagreement. |
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.
The text was updated successfully, but these errors were encountered: