-
Notifications
You must be signed in to change notification settings - Fork 23
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
Added konclude_test.py #231
base: master
Are you sure you want to change the base?
Conversation
I've also updated the evaluation.yml file. @sasjonge do you know how to write Docker files so that Konclude is also available? Another couple of things to fix: a) format. Konclude supports only OWL/XML and OWL functional syntax, not the RDF/XML that SOMA is written in. This needs an intermediate conversion step. |
I can have a look. I saw that konclude already has an existing docker image: https://hub.docker.com/r/konclude/konclude Maybe we can use that, or atleast their Dockerfile to create our own image. |
I tested using the konclude container and it seems to generally work: But if fails on this step, because a action is missing: Did you forget to push it? |
Not so much forgot, as asking for assistance :) As far as I understand, the Konclude exec should be placed at the appropriate place by some kind of container setup. Don"t know how to do that. |
Actually wait, now looking at your previous message I don't understand what the problem is. Line 99 seems to just call Konclude, and if that is already there then where is the problem? |
I think I got it to work. Here is how i call the classification: Here is the CI run: https://github.com/sasjonge/soma/runs/7199936734?check_suite_focus=true The "## Classes equivalent to http://www.w3.org/2002/07/owl#Nothing" of the evaluate step has a weird output: Is this normal: https://github.com/sasjonge/soma/runs/7199936734?check_suite_focus=true#step:8:11 |
So the the problem is this: 2022-07-05T16:01:22.3350599Z ## Classes equivalent to http://www.w3.org/2002/07/owl#Nothing ? I will have a look on my side. |
The other outputs are always: No issues detected. For "## Classes equivalent to http://www.w3.org/2002/07/owl#Nothing" it is:
|
Can I merge my changes to the konclude-ci branch to extends this PR/Draft? |
Pushed a commit that should fix this. The output will now be no issues detected if no classes are Nothing, and if there are unsatisfiable classes these will appear in a list. NOTE: Konclude cannot test emptiness of properties. I may have mentioned this before, but just to make sure we're all reminded of this, here it is again. This shouldn't affect us too much as the typical way for a property to be empty is for either its domain or range to be empty. As long as we don't use intersections when specifying domains/ranges, we should be fine. If we use named concepts, or unions of named concepts, to specify domains/ranges, then the emptiness of the property would be caught by emptiness of the domain/range concepts. |
Added docker run call to konclude
I merged my code into this branch wirh this PR #247 |
Good. If the ci now works, this is ready for review. |
Actually, it can via OWL link (I checked using my Server). Another option might be be to check for every ObjectProperty r whether Edit: The latter is not possible with OWL Link unfortunately - Konclude crashes 😅 |
"Yes". As long as I have a list of object properties r, then one thing to do is to create a "query owl file" containing concepts of the form "r some Thing" for each r, then do a classification. The one thing I'm not sure how to do right now is to get that list of properties. One option would be to check the list of declarations in the SOMA ugly file, assuming that all used object properties are also declared. |
It would not be a problem to generate such a list with the OWL API. |
Is this necessary to approve this PR? |
"Your" call -- for some value of "your". As I said, what this means is the code as-is right now will not detect impossible properties unless they have impossible domains. This is maybe sufficient given that most of the domains we define are not intersections. On the other hand, we may have intersection domains, indeed the new versions of some of the GCIs were rewritten as such. |
If you tell me where to put such a list of properties, I will gladly export it so that you can then read it :) |
Would the same folder housing the merged SOMA be ok? (I know I can access that one for Konclude input :) ) |
I would rather recommend to have the path as an passed argument, so that we don't have to change the file if we change the locations in the pipeline later. |
Yes, that looks okay. |
I now implemented exporting the properties. Just add a runtime argument propertyListPath to the java-ci stuff, e.g., as follows: spring-boot:run "-Dspring-boot.run.arguments=--versionInfo=current --propertyListPath=<path X>" This will create 2 files for each of the "main" ontologies (with that I mean the minimal set of ontologies, so that all ontologies from within the owl directory are imported - currently, these are Allen, Home and Elan. Interesting path problem to identify those btw). For example, |
@mrnolte I have a problem using the flag. I get a ¨Could not create dir" error in CI and also locally: https://github.com/sasjonge/soma/runs/7629898757?check_suite_focus=true#step:5:376 Also would it be possible to make the propertyExporter optional, so that the other CI steps don't need to execute it (and get the argument)? |
@sasjonge this should have fixed it, can you check? Also, if propertyListPath is not given, the CI will now skip it. |
Updated the script. It will now optionally take an argument that gives a path to a file containing a list of object properties, each with its full IRI, one property per line. The current line in the evaluation workflow reads: python ./scripts/konclude_test.py -k ./.github/actions/Konclude -s ./build/SOMA-HOME.owl -o ./owl/konclude.html ./build/konclude.output it should be python ./scripts/konclude_test.py -p -k ./.github/actions/Konclude -s ./build/SOMA-HOME.owl -o ./owl/konclude.html ./build/konclude.output Also please check if the paths to the Konclude binary (the -k arg) and the merged soma (-s) are correct. |
@mpomarlan I tried to change it in the CI, but if you now want to use the konclude binary from inside of your script it will make the setup more difficult, because I can't just use the konclude docker file. I'm wondering if we could change the system call in your script to a docker run call? |
I don't know how to do that, but I suppose it should be fine. Can you implement it? |
I tried to call the docker image from the python script on my fork (still a bit hacky with the pathes): This did run through: https://github.com/sasjonge/soma/actions/runs/3337441052/jobs/5523785507 I printed out the SOMA_QUERY.owl in that CI. Should there only be ObjectSomeValuesFrom axioms in that file? Also the runtime is 0ms. |
The 0ms likely comes from this: File '/data/SOMA-HOME.owl' not found. When called, the Konclude binary needs to have access to both the SOMA_QUERY and SOMA_HOME. |
About SOMA_QUERY, the structure seems ok (I didn't notice Konclude complaining about parsing errors) and the content is intentional: when querying for properties, I only care about concepts of the form hasProperty some Thing (if hasProperty is equivalent to Bottom, then its domain is Nothing). More interesting is that some properties have IRIs containing something else than SOMA or DUL, e.g. http://www.ease-crc.org/ont/SOMA-OBJ.owl#hasChildLink. This is coming from the property list file I imagine. |
@mrnolte Is that with this different IRI's a problem with the script? |
@mpomarlan
My newest run is this: https://github.com/sasjonge/soma/actions/runs/3338066719/jobs/5525234865 |
It is a problem that @daniel86 explained to me: Someone who worked on SOMA-OBJ did not configure protege correctly and created all kinds wrong iris. His old script renamed these. I wondered whether old neems use the SOMA-OBJ IRI and did not change my script to also rename those IRIs, but that should be no problem after my vacation. |
The xml parsing error is not important as this is not an xml format, but the OWL functional syntax parsing error is. I think this is because the property list file has property IRIs with <> around the names whereas I assumed they would be without. If this is it then the fix is simple I'll have it done in a minute. |
Should be done now. |
Seems like a command line arg is wrong now, SOMA_HOM_objectProperties not found. |
(Work in progress)
Part of addressing #219
After the konclude_test.py script is accepted, the evaluation.yml file needs to be changed, and the docker configuration file too.
What else should be changed to update the CI?