Open jeswr opened 2 years ago
I just wanted to jot a few top-of-mind notes around a couple of modifications that should be made down the line to improve the features & performance of reasoning.
I'm not suggesting we do this now.
true
/false
as the value to indicate implicit/explicit rather than using null
. This is my preference - but it would result in a slowdown if you are only trying to look up explicit, or only trying to look up implicit quads.add
/delete
is run). Though this has the potential for users to necessarily cause unnecessary slowdowns if they choose to repeatedly call delete
on single quads rather than batching quads due to the nature of incremental maintenance algorithmshas
and match
calls. In particular, this may suggest that we should go with the suggestion of extending the store rather than using composition.@jeswr I would love to see this code in N3. I have immediate plans to use this in some webapps I have in mind.
Do you have in the reasoner support for native N3 lists? In my own attempts I've struggled with that. Dörthe Arndt created a test case for me a while ago. How would this be expressed in the N3 Reasoner?
PREFIX : <http://example.org/socrates#>
:Socrates a :Man.
:Man rdfs:subClassOf :Mortal.
_:x a :Man.
{
?A rdfs:subClassOf ?B.
?S a ?A.
} =>
{
?S a ?B
}.
{
?x a ?y.
?x a ?z.
}
=>
{
?x :aa (?y ?z)
}.
I would love to see this code in N3.
As would I, however, I am currently undecided on whether the API should be as it is at present, or, whether N3Reasoner
should extend N3Store
and supply a .reason
method. I think for the sake of supporting things like incremental reasoning (as mentioned here) it should actually be the latter (@rubensworks do you have a view on this given your considerable work on the RDFJS spec?).
That said it shouldn't be much work for you to change between the two usages so you could always just use the branch in its current state in your app by running npm i https://github.com/jeswr/N3.js/tree/feat/N3Reasoner
or npm i https://github.com/rdfjs/N3.js/tree/a13e50e20c18eb0bc869db2a7c09fe552c653a4e
Do you have in the reasoner support for native N3 lists?
Not in the current state of this PR, as that would require the capability to be able to mint new blank nodes in order to represent the list.
Adding this capability shouldn't be too difficult, but I would prefer to look into how other reasoners do it and think about the best way of encoding that in rules so as to not limit ourselves from future performance improvements.
@jeswr thanks this works. I see that the reason
method inline updates the N3Store. Would it be possible to also support that the reason method returns a new N3Store (with the new produced quads) while keeping the old one intact.
let newStore = N3.Reasoner(oldStore).reason([ { premise:... , conclusion: ... } ]);
As would I, however, I am currently undecided on whether the API should be as it is at present, or, whether N3Reasoner should extend N3Store and supply a .reason method. I think for the sake of supporting things like incremental reasoning (as mentioned https://github.com/rdfjs/N3.js/pull/296#issuecomment-1126698258) it should actually be the latter (@rubensworks do you have a view on this given your considerable work on the RDFJS spec?).
+1 on N3Reasoner
. However, instead of making this an extension of N3Store
, it might be more interesting to use composition for this, as the latter is more flexible. This would also move more in the direction of making this reasoning logic more generic at RDF/JS-level in the future.
Edit: I see the current approach already uses composition, which is good IMO :-)
@jeswr thanks this works. I see that the
reason
method inline updates the N3Store. Would it be possible to also support that the reason method returns a new N3Store (with the new produced quads) while keeping the old one intact.let newStore = N3.Reasoner(oldStore).reason([ { premise:... , conclusion: ... } ]);
It's certainly feasible, though it may incur a performance hit. I'll see if I can give this a go over the weekend (no promises though). There are, again, some API decisions to be made around this - but at the very least I would consider adding support for multiple stores as input at the same time as this would require the same additions to the logic, as that required to apply the sem-naive algorithm across the oldStore
and the newStore
.
This introduces an N3 Reasoner class as discussed in #295
For the record here is the output of the performance test, noting that my machine has ~20% variance in results as noted in past PRs