Spatial conclusion #63

ctieben · 2018-12-17T11:05:41Z

Changes:

Sperate SpatialIndex for 2D and 3D. This will fix Boost Geometry and GOES::Geom conflict in Spatialindex #55
Add a SpatialConclusion that will generate RDFTriple for geometry relations.
Also the redudant document store in the triple is changes, fixed Triple::document redundant #33
The RDF Entity could be temporary
GlobalCS now knows the cardinal direction.
isEqual now for all SpatialReference and not only for GlobalCS
Add additional SpatialQuery types
Add converter for Eigen, Boost and GEOS::GEOM points in the SpatialIndexBase

Currently the temp entities uses valid IDs and will wast them.

…riple struct. This will close #33

…DFVector. Also the RDFVector could now be temporary.

…atialIndex for different reference systems possible

…ry pair as reference

…s, contains and within conditions.

… SPARQL Query

…branch

…ry pair as reference

…s, contains and within conditions.

… SPARQL Query

…branch

…ersion and log infos

niniemann

Phew, a lot has happened since I last looked into sempr. 😅
There are some things that definitely should be addressed:

The whole point of GeometricObject is not yet clear to me
registerProperty has been abused by the GeometricObject beyond all good and evil
The strange behaviour of findEnvelope you mentioned should be investigated
The geometry checks in the SpatialIndex might expose unintuitive/unwanted behaviour in the 3D case (because the "more precise" geometry checks are 2D)

Plus some minor, less important remarks, but those should be looked into as well. :)
Nevertheless, thanks for all the contributions, and sorry for the late review!

niniemann · 2019-02-27T11:10:57Z

include/sempr/core/RDF.hpp

-#   undef RDFS
-}
+        namespace rdfs {
+            const std::string& baseURI();


Why did you add this line? const std::string& baseURI(); is already declared through the macro below:

#define RDFS(name) const std::string& (name)(); RDFS(baseURI)

Same goes for owl.

niniemann · 2019-02-27T11:13:25Z

include/sempr/entity/GeometricObject.hpp

+
+
+#pragma db object
+class GeometricObject : public SemanticEntity {


Some comments would be nice. What is the purpose of this class?

niniemann · 2019-02-27T11:26:49Z

include/sempr/entity/GeometricObject.hpp

+    const std::string type() const { return type_; }
+    void type(const std::string& type);
+
+    void registerProperty(const std::string& predicate, const std::string& object);


A method of the same name (but templated) already exists in SemanticEntity, which you derive from here. Is this on purpose? Or coincidental?

template <class T> void registerProperty(const std::string& predicate, T& property) { /* ... */ } // ... and derivations ...

These methods are used to register member variables and make them visible in the rdf.

niniemann · 2019-02-27T12:30:08Z

include/sempr/entity/RDFVector.hpp

    virtual ~RDFVector(){}

    void getTriples(std::vector<Triple>& triples) const;
    const Triple& getTripleAt(const size_t index);
-    bool addTriple(const Triple& triple);
+    bool addTriple(const Triple& triple, bool replace = false);     // if replace than an equal triple will be removed before, otherwise there a multiple entries possible


What's the meaning of "replacing" an equivalent Triple? If there's a way to see a difference they were not equal. So replace = true would be the same as allow_duplicates = false. The other way round is still intuitive for allow_duplicates, but replace = false is ambiguous: What happens with an equivalent triple? Is it just discarded without "replacing" the old one, and there is still only one of present in the vector? Or is it added as a duplicate?

tl;dr: I'd suggest renaming bool replace to bool allow_duplicates.

Also, maybe the default should be to not allow duplicates? I don't see any use case for adding the same fact multiple times.

niniemann · 2019-02-27T12:46:33Z

include/sempr/entity/RDFVector.hpp

@@ -48,11 +43,15 @@ class RDFVector : public RDFEntity {
    TripleIteratorWrapper begin() const override;
    TripleIteratorWrapper end() const override;

+    bool validity(const Triple& triple) const;


What does this check? Does it depend on the RDFVector, or should this be moved to the Triple-class?

niniemann · 2019-02-27T14:37:25Z

src/entity/GeometricObject.cpp

+    for(auto t : *this)
+    {   
+        // empty predicate will add all related object to the result list
+        if ( predicate.empty() || t.predicate == predicate)


Again, I don't really understand what this is meant to do. 🤔
You have triples with an empty predicate stored in this? Why?

niniemann · 2019-02-27T14:40:08Z

src/entity/RDFVector.cpp

 {
+    Triple t = triple;


Why the temporary object & copy here?

niniemann · 2019-02-27T14:45:33Z

src/entity/SemanticEntity.cpp

+    return  hasSubject_ == other.hasSubject_ &&
+            subject_ == other.subject_ &&
+            predicate_ == other.predicate_ &&
+            object() == other.object() ;


This operator only checks if the subjects and predicates equal and the objects currently have the same value. To really check if the RegisteredPropertys are equal (i.e., refer to the same member variable) you would need to check the address of the T& value_ in the derived class. But that's obviously not so easy. And I still think this is born from the desire to misuse the SemanticEntity. 😉

niniemann · 2019-02-27T14:55:01Z

src/processing/CalculatePointsModule.cpp

    ask(spatial);
    entity::PointCloud::Ptr entity = entity::PointCloud::Ptr(new entity::PointCloud());

    for(auto& c : spatial->results)
    {
-        if(c->discriminator() == "sempr::entity::PointCloud")
+        if(c->discriminator() == "sempr::entity::PointCloud")   // could this be done by the dynmaic cast?


A nicer version would be to not hardcode the discriminator, but use the odb type traits:

if (c->discriminator() == odb::object_traits_impl<sempr::entity::PointCloud, odb::id_common>::info.discriminator)

should work. (Though this is still a check for exactly PointClouds. This does not match any derived entities. Just sayin').

niniemann · 2019-02-27T14:58:28Z

test/main.cpp

@@ -85,7 +85,7 @@ int main(int argc, char** args)
    DebugModule::Ptr debug(new DebugModule());
    ActiveObjectStore::Ptr active( new ActiveObjectStore() );
    SopranoModule::Ptr semantic( new SopranoModule() );
-    SpatialIndex::Ptr spatial( new SpatialIndex() );
+    SpatialIndex3D::Ptr spatial( new SpatialIndex3D(std::make_shared<LocalCS>()) );


How can this work? I thought entities need to be located relative to the coordinate system given to the index, in order for the index to work?

niniemann · 2021-08-13T11:47:59Z

More than stale. (Development proceeded in an internal git).

ctieben added 30 commits September 5, 2018 16:30

Allow temporary entites.

ca78e62

Currently the temp entities uses valid IDs and will wast them.

Add a RDFValue based Triple implementation and remove document from t…

cbbe37b

…riple struct. This will close #33

Add a RDFValue based variant of a Triple that is now be used in the R…

d9b28f6

…DFVector. Also the RDFVector could now be temporary.

Remove RDFValueTriple Try

aa1d520

isEqual check for all ReferenceSystems

a624097

Fix isEqual for no parent of a local cs

968dd6e

SpatialIndex now for a specific root reference system. \n Multiple Sp…

4a25848

…atialIndex for different reference systems possible

First Version of SpatialConclusion

c9306ee

SpatialConclusion as SpatialIndex Extension

2b4fcc5

R-Tree with cloned geometries

646620b

SpatialConclusion could also checks geometry and not only boxes

5f837b5

R-Tree only for query not iterating

ce85470

Fix 2D in SpatialIndex by assume a very high box

183aeed

Add first tests for spatial conclusion

7ed1837

SpatialIndexQuery for 2D or 3D by template parameter and a box/geomet…

593402e

…ry pair as reference

SpatialIndex separate for 2D and 3D

796867f

Fix static assert from eigen in SpatialIndexQuery2D

17b7f3b

Move type mapping and QueryType to SpatialIndexBase

a6b2f98

Add support for Nearest, CoveredBy and Overlaps spatial queries.

9619bd6

Allow Geometry based spatial queries (not Box based) for intersection…

eba14d7

…s, contains and within conditions.

Fix geometry check for Spatial Queries and update unit test

d27e690

SpatialConclusion for 2D and 3D. Add SpatialThing as example entity.

5a42d2c

Fix shortcut in RDF Triple and add test for a SpatialConclusion based…

747838b

… SPARQL Query

check URI and RDF Shortcut in SpatialConclusion

6c34475

Add generic cardinal direction checker for WGS84

dfb98c9

DirectionCheck now supportes different global coordinate systems

3b170a5

Add removeProperty for SemanticEntity

5d6a513

Rename and move SpatialThing to GeometricObject

a50f627

GeometricObject and SemanticEntity may be temporary and clean up the …

565de92

…branch

Merge remote-tracking branch 'origin/master' into spatial_conclusion

c56fe03

ctieben added 21 commits October 25, 2018 17:34

Add first tests for spatial conclusion

e5cf03e

SpatialIndexQuery for 2D or 3D by template parameter and a box/geomet…

c26e1c0

…ry pair as reference

SpatialIndex separate for 2D and 3D

0d4c7b5

Fix static assert from eigen in SpatialIndexQuery2D

e8363c8

Move type mapping and QueryType to SpatialIndexBase

4b873f5

Add support for Nearest, CoveredBy and Overlaps spatial queries.

b60d0b3

Allow Geometry based spatial queries (not Box based) for intersection…

cc7bc18

…s, contains and within conditions.

Fix geometry check for Spatial Queries and update unit test

af716a0

SpatialConclusion for 2D and 3D. Add SpatialThing as example entity.

432cfe1

Fix shortcut in RDF Triple and add test for a SpatialConclusion based…

5b191e1

… SPARQL Query

check URI and RDF Shortcut in SpatialConclusion

28e89b3

Add generic cardinal direction checker for WGS84

f0c0dc5

DirectionCheck now supportes different global coordinate systems

ff818ef

Add removeProperty for SemanticEntity

9444e99

Rename and move SpatialThing to GeometricObject

8dc12fb

GeometricObject and SemanticEntity may be temporary and clean up the …

a2c79f6

…branch

GeometricObject will not notify about changes of himself

ff42d68

Update SpatialConclusion comments

fb4328d

Merge master into spatial_conclusion

fd09057

Merge branch 'master' into spatial_conclusion

0ccb17d

Fix CalculatePointsModule for changed SpatialIndex and update cmake v…

b5e47bc

…ersion and log infos

ctieben requested review from y0urself and niniemann December 17, 2018 11:05

Update comment

6be6b22

niniemann requested changes Feb 27, 2019

View reviewed changes

ctieben added 2 commits March 1, 2019 11:35

Merge branch 'master' into spatial_conclusion

617adf4

Fix nullptr exception in calculate pointclouds

cf1bee1

niniemann mentioned this pull request Jul 10, 2019

optional param for temporary RDFVector #69

Merged

niniemann closed this Aug 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spatial conclusion #63

Spatial conclusion #63

ctieben commented Dec 17, 2018 •

edited

Loading

niniemann left a comment

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann Feb 27, 2019

niniemann commented Aug 13, 2021



		#pragma db object
		class GeometricObject : public SemanticEntity {

Spatial conclusion #63

Spatial conclusion #63

Conversation

ctieben commented Dec 17, 2018 • edited Loading

niniemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niniemann commented Aug 13, 2021

ctieben commented Dec 17, 2018 •

edited

Loading