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

Integrate SADL List grammar with Jena builtins #630

Open
crapo opened this issue Mar 15, 2021 · 11 comments
Open

Integrate SADL List grammar with Jena builtins #630

crapo opened this issue Mar 15, 2021 · 11 comments
Assignees

Comments

@crapo
Copy link
Collaborator

crapo commented Mar 15, 2021

See RuleWithList.sadl in TestSadl3Ide example project.

@crapo crapo self-assigned this Mar 15, 2021
@crapo crapo changed the title Integrate SADL grammar "length of <List>" with builtin listLength Integrate SADL List grammar with Jena builtins Mar 17, 2021
crapo pushed a commit that referenced this issue Mar 19, 2021
tuxji added a commit that referenced this issue Mar 21, 2021
PR for issue Gh 630: implementation of list built-ins for Jena reasoner
tuxji added a commit that referenced this issue Mar 25, 2021
added missing list built-ins to insert, delete; also added sadlListTo…
crapo pushed a commit that referenced this issue Mar 26, 2021
crapo pushed a commit that referenced this issue Mar 26, 2021
tuxji added a commit that referenced this issue Mar 26, 2021
@crapo crapo mentioned this issue Mar 30, 2021
tuxji added a commit that referenced this issue Mar 31, 2021
Fixed conflicts:
	sadl3/com.ge.research.sadl.parent/com.ge.research.sadl.jena/src/com/ge/research/sadl/jena/JenaBasedSadlModelValidator.java
@crapo
Copy link
Collaborator Author

crapo commented Apr 5, 2021

@agabaldon , @AbhaMoitra , I would like to make some changes to lists in SADL. I'm not sure how much you have used this recently implemented functionality or set of built-in functions, so I'm bringing it up for discussion before committing changes.

  1. It was a mistake to call the JenaReasonerPlugin's list built-in "length" as that is a term that is a) a keyword in SADL, and b) a common term that modelers might like to be able to use in SADL without having to always prefix its (QName) because it is in the SadlBuiltinModel (at least for Jena-based reasoners). Rather it should be named "listLength", like the Jena-provided built-in for RDF lists.
  2. The "listLength" built-in function can easily handle both SADL typed lists and RDF lists. In fact, all of the list built-ins could easily be enhanced to handle either type of list.
  3. There is a bigger potential change. I've probably gotten carried away with the imagined importance of typed lists. The real value of typed lists, presumably, is just that--they are typed, enabling type checking of their content. There is no reason that the SADL typed list needs to have its own "first" and "rest" properties; it could use the rdf:first and rdf:rest properties and simply add the property restrictions on these properties when the list is typed. It may be possible to tweak the SADL grammar to allow regular RDF lists to be created in SADL--that would require some experimentation to determine. If that is possible then it would allow a modeler to create lists in SADL, as well as reason on any kind of list, that contain a mixture of any kind of items--numbers, instances of different classes, etc.

Thoughts? I will not make changes before hearing from you on this topic.

@AbhaMoitra
Copy link
Collaborator

@crapo : I am ok with these changes and they make sense.

@agabaldon : can you also take a look at this and share your views.

@AbhaMoitra
Copy link
Collaborator

@crapo : I am trying to use 'length of a list" and I am not able to get it working. Maybe my syntax is in correct? I am using

and len = builtinfunctions:^length(n2)

I also tried

and len = ^length(n2)

@crapo
Copy link
Collaborator Author

crapo commented Apr 7, 2021

@AbhaMoitra , in my test case I use:
Rule ListLength:
if l1 is a YooHooList and len is length of l1 then print("Length of l1: ", len).

which translates to the Jena rule:
[ListLength: (http://sadl.org/UsingListExpression.sadl#l1 rdf:type http://sadl.org/UsingListExpression.sadl#YooHooList), listLength(http://sadl.org/UsingListExpression.sadl#l1, ?len) -> print('Length of l1: '^^http://www.w3.org/2001/XMLSchema#string, ?len)]

but this is on a branch of the code where I have changed the name of the built-in to listLength. I'm not sure I have a test case where built-in "length" was used as an explicit function rather than with the SADL grammar.

@agabaldon
Copy link
Collaborator

@crapo: the change to listLength makes sense. I think the flexibility you describe in (3) would also be good to have in the future.

What branch should I install to test listLength?

@crapo
Copy link
Collaborator Author

crapo commented Apr 7, 2021

@agabaldon , I just created a PR on DefaultValues branch that has both default values and new "listLength".

@AbhaMoitra
Copy link
Collaborator

@crapo : Here is my rule - rewritten to use your syntax style above (I am using https://github.com/ml4ai/grasen-data/blob/master/SemanticModels/GraSEN

Rule MeasuredSignalCandidate // error signal = reference signal - measured signal
if node is a Function
and nodes of node is n1
and children of n1 is n2
and len is ^length of n2
then measuredSignalCandidate of node is true
and print("in MeasuredSignalCandidate, node is ", node).

In the Rules file, the syntax for "length" clause is

(?n2 http://sadl.org/builtinfunctions#length ?len)

Is that what you expect?

@crapo
Copy link
Collaborator Author

crapo commented Apr 7, 2021

@AbhaMoitra , that syntax indicates that the translation is treating "length" as a property.

I'll restate my rule as I would expect it if the function name were "length" rather than "listLength".

[ListLength: (http://sadl.org/UsingListExpression.sadl#l1 rdf:type http://sadl.org/UsingListExpression.sadl#YooHooList), length(http://sadl.org/UsingListExpression.sadl#l1, ?len) -> print('Length of l1: '^^http://www.w3.org/2001/XMLSchema#string, ?len)]

For your rule, I'd expect:

... length(http://sadl.org/UsingListExpression.sadl#l1, ?len)...

I'm not sure what version of the code you are using. However, I see that Alfredo just merged the latest PR, which uses "listLength" as the Jena built-in function. Perhaps you should get the latest version of the SADL plugins that will be created from this merge into the development branch.

Note that you should delete the SadlBuiltinFunctions.sadl file from the ImplicitModel folder and let it be replaced with a new one so that function names are updated.

@agabaldon
Copy link
Collaborator

@crapo: after pulling the most recent changes, using listLength is not working but length is. Are we missing some update?
Also, I see some message about "Derivations written to..." Should we turn some setting off?

@crapo
Copy link
Collaborator Author

crapo commented Apr 9, 2021

@agabaldon , I'm going to guess that this is because the old SadlBuiltinFunctions.sadl has not been deleted and still contains "length" rather than "listLength". It is also possible that the configuration.rdf file, which contains the fully qualified names of built-in function classes for Jena-based rules might be causing an error message. Currently the SadlBuiltinFunctions.sadl file is replaced when reasoner and/or translator selection is changed. I don't see any downside to deleting it as part of a clean as build will replace it with a new instance freshly created by the selected translator/reasoner pair.

The "Derivations written... " is a regression bug. I will fix it.

@crapo
Copy link
Collaborator Author

crapo commented Apr 9, 2021

@agabaldon , @AbhaMoitra , just created a PR that addresses this and several other issues.

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

3 participants