opencaesar / oml

Ontological Modeling Language (OML)
https://opencaesar.github.io/oml/
Apache License 2.0
23 stars 4 forks source link

[BUG] - incorrect logic in OmlSearch.findInstancesRelatedTo #103

Open NicolasRouquette opened 1 year ago

NicolasRouquette commented 1 year ago

Description

A clear and concise description of what the bug is.

The implementation does not conform to the documented intent.

Steps to Reproduce

Steps to reproduce the behavior:

If instead of using https://github.com/NicolasRouquette/computational-hypergraph/blob/270fb0820df8a2d7da82ec0cbfeaaac11c2d888b/viewpoint/src/edu/caltech/ch/OmlServices.java#L149-L161, the logic where to use OmlSearch.findInstancesRelatedTo, then the diagrams would have the incorrect contents.

Expected Behavior

A clear and concise description of what should be the expected behavior.

Additional Context

Enter any other details such as dependencies, environment, examples, etc.

Relevant screenshots

If applicable, add screenshots to help illustrate the issue.

Correct contents for nested1 diagram:

image

Correct contents for xt diagram:

image

melaasar commented 1 year ago

I agree the OmlSearch.findInstancesRelatedTo(NamedInstance target, Relation relation) should match the forward (not the reverse) relation of the relation instances' types.

However, I wonder if we have the following description:

ci component1 : mission:Component [
   mission:performs function1
]
ci function2 : mission:Function [
   mission:isPerformedBy component1
]
ri perform1 : mission:Performs [
   from component1
   to function3
]

Currently, a call to OmlSearch.findInstancesRelatedTo(function1, performs) would return component1, a call to OmlSearch.findInstancesRelatedTo(function3, performs) would return component1, where a call to OmlSearch.findInstancesRelatedTo(function2, performs) would return no component. Is this expected?

Conversely, a call to OmlSearch.findInstancesRelatedTo(component1, isPerformedBy) would return function2 only. Should it also return function1 and function3?

More generally:

melaasar commented 1 year ago

@NicolasRouquette I fixed the API to match the forward relation.

However, would appreciate your input on the comment above.