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

Merging outstanding Pull Requests and gjhiggins' branch #15

Open
wwaites opened this issue Jan 11, 2017 · 16 comments
Open

Merging outstanding Pull Requests and gjhiggins' branch #15

wwaites opened this issue Jan 11, 2017 · 16 comments

Comments

@wwaites
Copy link
Member

wwaites commented Jan 11, 2017

Is there any good reason why not to merge these in? There's quite a lot of fixing needs done, may as well pull in the work that people have been doing?

@wwaites
Copy link
Member Author

wwaites commented Jan 11, 2017

Further to this, I propose that

  1. This become the "official" repository if Chimezie is happy with that, and
  2. We turn off Travis-CI temporarily.

The reason for #2 is that it prevents merging pull requests while the tests are not passing. There is a substantial amount of work to be done to get the tests passing and this situation prevents incremental fixes. It is very hard to move forward if a single giant fix of everything has to be made before anything else can be done.

@gromgull
Copy link
Member

From the mailing-list activity and support request, I guess Fuxi has pretty much no users, and certainly no maintainers.

There is more detail here:
#9

Me and Chimezie had an exchange a few years back about re-merging layercake and rdflib, both were keen for it to happen, but no one had any time to work on it :(

If you have a need for Fuxi and you are keen to maintain it @wwaites I am happy to let you have admin access to this repo and you can merge and fix whatever you want!

Making the bits that worked with the old RDFLib Sparql 1.0 engine work with the newest RDFLib is probably a non-trivial job though!

@wwaites
Copy link
Member Author

wwaites commented Jan 11, 2017

I do have a need for it, but right now only a corner of it (the forward-chaining). This actually still works and is very useful for my needs, which are writing short, succinct paragraphs of RDF and using rules to generate more verbose documents that can then be used directly in Python. This trick was what first led me to FuXi years ago and it still works very well.

Right now, it looks like the version we have here is non-functional and the version with @gjhiggins changes is, so I would be happy to do that, together with some minor changes that I've made to get some more of the tests running (low hanging fruit). I agree that porting the back-chaining SPARQL store is not trivial. Fixing InfixOWL is probably not too hard, I think it was always more or less stand-alone anyways. This is more a commitment to take over curation and minor fixes from Graham rather than proper maintainership because my plate is very full at the moment.

Much more encouraging, I was discussing with @chimezie in IRC and he mentioned possibly wanting to take it back up.

@gromgull
Copy link
Member

@wwaites any curation you can do is welcome! You are now admin!

The challenge with rdflib and layercake reunification is that layercake has the old sparql engine, and they rely on the internals to implement parts of the evaluation efficiently in MySQL. Again, porting it is non-trivial.

@chimezie
Copy link
Member

Hello.

I'm happy with this becoming the "official" repository and my recent work is making its way back into the semantic web space. So, I anticipate having more time available to help with a renewed attempt to merge the two projects.

I will keep an eye on this thread.

@westurner
Copy link

Is there yet a better way to do N3 or OWL in Python?

@westurner
Copy link

westurner commented Oct 13, 2021

Is there a meta-issue listing what all needs to be done to modernize FuXi?

FWIU:

  • SPARQL 1.1
  • rebase and merge (?) Layercake onto the now-canonical GitHub repo?
    • is that a potentially long-running feature branch or is there a more piecemeal alternative that wouldn't block all of these other developments in the FuXi project over the years?

@nicholascar
Copy link
Member

This might be interesting to see revived!

@ghost
Copy link

ghost commented Nov 12, 2021

Wellll ... as regards a meta-listing, I can initially offer a (gzipped, d/l and open in browser) junit2html rendering of the nose test results (see attachment) after I went through removing the obsolete references to the pycompat support (that used to exist as a separate module in rdflib) and imposing a weak-ish flake8 regime.

As far as I can make out, quite a lot of the basic support code remains functional but some of the key components are not working properly - for example, a fairly straightforward test of built-in ordering

def test_uriref_variable_startswith_literal_should_match(self):
fails because the networks returned by both NetworkFromN3 and ReteNetwork are empty.

As observed upthread, non-trivial breaking changes were introduced in the migration from the old bison-based SPARQL implementation, these profoundly affect the BackwardChainingStore implementation, for examples, see

def isaBaseQuery(self, queryString, queryObj=None):
where the query is parsed and
# isinstance(_expr, BasicGraphPattern):
where the results of that parsing are used to drive analysis - the code is commented as: “The default 'native' SPARQL implementation is based on sparql-p's expansion trees layered on top of the read-only RDF APIs of the underlying store”.

Unfortunately, Chimezie advised at the time that the ForwardChainingStore was deprecated (for what I would imagine are unimpeachable reasons).

There's more to say about the various tests - I'll assemble my thoughts more coherently and attach a document here for perusal. In the interim I will commit my update to a separate branch in my fork (https://github.com/gjhiggins/FuXi).

And props to Nick for a solid update of InfixOWL :)

nosetests.html.gz

@ghost
Copy link

ghost commented Nov 12, 2021

Interim progress report:

In the interim I will commit my update to a separate branch in my fork (https://github.com/gjhiggins/FuXi).

Now done.

https://github.com/gjhiggins/FuXi/tree/maybe

ATM, I'm running this in a venv with tox -e py3

Nearly all of the failing tests are because of inoffensive result ordering mismaches and, once recorded, can be elided with a doctest SKIP, leaving just the important exception of the mangled rules:

File "/FuXi/Rete/SidewaysInformationPassing.py", line 300, in FuXi.Rete.SidewaysInformationPassing.BuildNaturalSIP
Failed example:
    for rule in rs: print(rule)
Expected:
    Forall ?Y ?X ( ex:sg(?X ?Y) :- ex:flat(?X ?Y) )
    Forall ?Y ?Z4 ?X ?Z1 ?Z2 ?Z3 ( ex:sg(?X ?Y) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )
Got:
    Forall ?Y ?X ( And(  ) :- ex:flat(?X ?Y) )
    Forall ?Z3 ?X ?Y ?Z1 ?Z2 ?Z4 ( And(  ) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )

where the replacement of ex:sg(?X ?Y) by ( And( ) :- appears to be related to the as-yet-unaddressed issue of:

SyntaxWarning: Integrity constraints (rules with empty heads) are not supported: 
Forall ?det ?infName ( And(  ) :- And( ns1:Detection(?det) ns1:name(?det ?infName) ) )

Looks like that shouldn't be happening.

@ghost
Copy link

ghost commented Nov 12, 2021

Now done.

https://github.com/gjhiggins/FuXi/tree/maybe

Issues and Discussions enabled for the above fork, then upgrade chatter doesn't contaminate the "official" repos.

@nicholascar
Copy link
Member

@westurner do you understand the operations of FuXi enough to be able to review @gjhiggins recent updates linked to above?

@ghost
Copy link

ghost commented Nov 14, 2021

Another interim progress report:

Nearly all of the failing tests are because of inoffensive result ordering mismaches and, once recorded, can be elided with a doctest SKIP, leaving just the important exception of the mangled rules:

File "/FuXi/Rete/SidewaysInformationPassing.py", line 300, in FuXi.Rete.SidewaysInformationPassing.BuildNaturalSIP
Failed example:
    for rule in rs: print(rule)
Expected:
    Forall ?Y ?X ( ex:sg(?X ?Y) :- ex:flat(?X ?Y) )
    Forall ?Y ?Z4 ?X ?Z1 ?Z2 ?Z3 ( ex:sg(?X ?Y) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )
Got:
    Forall ?Y ?X ( And(  ) :- ex:flat(?X ?Y) )
    Forall ?Z3 ?X ?Y ?Z1 ?Z2 ?Z4 ( And(  ) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )

I've made a couple of fixes and now it “correctly” fails :)

File "lib/Rete/SidewaysInformationPassing.py", line 300, in lib.Rete.SidewaysInformationPassing.BuildNaturalSIP
Failed example:
    for rule in rs: print(rule)
Expected:
    Forall ?Y ?X ( ex:sg(?X ?Y) :- ex:flat(?X ?Y) )
    Forall ?Y ?Z4 ?X ?Z1 ?Z2 ?Z3 ( ex:sg(?X ?Y) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )
Got:
    Forall ?X ?Y ( ex:sg(?X ?Y) :- ex:flat(?X ?Y) )
    Forall ?Z1 ?Y ?Z4 ?X ?Z2 ?Z3 ( ex:sg(?X ?Y) :- And( ex:up(?X ?Z1) ex:sg(?Z1 ?Z2) ex:flat(?Z2 ?Z3) ex:sg(?Z3 ?Z4) ex:down(?Z4 ?Y) ) )

@ghost
Copy link

ghost commented Nov 14, 2021

Okay, got this far:

Ran 90 tests in 2.054s

FAILED (SKIP=5, errors=1, failures=1)

Both the error and the failure arise from the profound issues with TopDownSPARQLEntailingStore still referencing obsolete functions in the ancient rdflib bison-based SPARQL implementation (as discussed above). Refactoring this to use RDFLib's current implementation would be non-trivial. However, the top-down approach has been deprecated so maybe this issue can be partially elided.

To spare those not deeply involved, I'll shift the rest of this discussion to the Discussions on my fork as it's going to get extensive in scope and specific in detail.

@westurner
Copy link

@nicholascar I wouldn't say that I am sufficiently familiar with n3 (=>: implies) or owl or the FuXi codebase to review this one, though I was able to figure out how to materialize the implied triples with FuXi in 2010 IIRC.

I find it surprising that the original author is unavailable for comment.

@ghost
Copy link

ghost commented Nov 14, 2021

I find it surprising that the original author is unavailable for comment.

Chimezie has probably moved on in the years since and is unlikely to be following developments.

BTW, after getting a better understanding of what needed to be done, I was able to re-do the fork and make the changes in a less monolithic manner.

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

No branches or pull requests

5 participants