Skip to content
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

N3 Reasoner Class #296

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

N3 Reasoner Class #296

wants to merge 8 commits into from

Conversation

jeswr
Copy link
Collaborator

@jeswr jeswr commented May 13, 2022

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

jeswr@debian:~/semi/N3.js$ NODE_ENV=production node --max-old-space-size=8192 perf/N3Reasoner-perf.js 
Reasoning over TimBL profile and FOAF
loading foaf ontology: 15.945ms
loading tim berners lee profile card: 13.481ms
Reasoning: 48.471ms

Running Deep Taxonomy Benchmark
Reasoning: test-dl-10.n3: 0.231ms
Reasoning: test-dl-100.n3: 1.019ms
Reasoning: test-dl-1000.n3: 26.149ms
Reasoning: test-dl-10000.n3: 76.392ms
Reasoning: test-dl-100000.n3: 1.035s
Reasoning: test-dl-1000000.n3: 10.606s

@jeswr
Copy link
Collaborator Author

jeswr commented May 14, 2022

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.

  • Track which quads are implicit vs. explicit. There are 2 ways I can immediately think of to do this
    1. In each dictionary use 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.
    2. Having 2 separate sets of indexes, 1 for implicit facts, the other for explicit. This is more likely to allow duplication of data (with explicit facts likely also to turn up in the implicit indices), will result in slower reasoning times due to having to look over 2 sets of indices, will result in slower lookup times for all data, again because you have to look over 2 sets of indices.
  • Handle incremental reasoning. There are several facets to this
    1. Allow the loading and preservation of rules
    2. The option to enable/disable incremental reasoning by default (i.e. should incremental reasoning be run every time 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 algorithms
    3. Have an attribute to indicate whether the current database is consistent with the rules and explicit data.
  • Backwards chaining (c.f. Reasoning using the semi naive algorithm #295 (comment)) - This option may mean we need to rethink the design of this class slightly so that we can hook into has and match calls. In particular, this may suggest that we should go with the suggestion of extending the store rather than using composition.

const [val0, val1, val2] = rule.premise[i].value, index = content[rule.premise[i].content];
const v0 = !(value = val0.value);
for (value in v0 ? index : { [value]: index[value] }) {
if (index1 = index[value]) {
Copy link
Collaborator Author

@jeswr jeswr May 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for self when I get around to finish cleaning this up. Due to the way that N3.js handles deletions; this is only here for the case where value is defined and !(value in index). So we can reduce a bunch of unnecessary calcs by moving this check outside of the loop.

One option is

for (value in v0 ? index : value in index ? { [value]: index[value] } : {})
   ...

@phochste
Copy link

@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)
}.

See also: http://ppr.cs.dal.ca:3002/n3/editor/s/C688Vckv

@jeswr
Copy link
Collaborator Author

jeswr commented Jun 26, 2022

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.

@phochste
Copy link

phochste commented Jun 27, 2022

@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: ... } ]);

@rubensworks
Copy link
Member

rubensworks commented Jun 27, 2022

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 #296 (comment)) 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 :-)

src/N3Reasoner.js Outdated Show resolved Hide resolved
src/N3Reasoner.js Outdated Show resolved Hide resolved
@jeswr
Copy link
Collaborator Author

jeswr commented Jun 28, 2022

@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.

jeswr and others added 2 commits June 28, 2022 15:17
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants