Fix observe() method when called for SGVector/SGMatrix. #4626
Conversation
| T_SGMATRIX_INT32 = 24, | ||
| T_SGMATRIX_INT64 = 25, | ||
| T_UNDEFINED = 26 | ||
| }; |
There was a problem hiding this comment.
Do you think it would be worth adding all the combinations of containers and primitive types, or some macro that would do that? Because right now stuff like SGVector<bool> isn't included (whether or not we actually use it is another question...)
There was a problem hiding this comment.
I guess it would make sense if we were to use it somewhere in Shogun. For instance, if we find a machine which needs to emit SGVector<bool> then we will add it.
geektoni
force-pushed
the
feature/observable_framework
branch
from
e540d7e
to
42835a7
Compare
geektoni
force-pushed
the
feature/observable_framework
branch
2 times, most recently
from
756b353
to
939c907
Compare
geektoni
force-pushed
the
feature/observable_framework
branch
from
c9a2064
to
4bdea29
Compare
geektoni
force-pushed
the
feature/observable_framework
branch
2 times, most recently
from
c8b39fe
to
30ca2d6
Compare
…d is_sg_matrix." This reverts commit cb70da3.
* Override create_empty() inside MockObject.h to ensure proper testing.
geektoni
force-pushed
the
feature/observable_framework
branch
from
30ca2d6
to
d2bce30
Compare
|
@karlnapf any final comments before I merge this? :) |
| @@ -316,6 +316,13 @@ namespace shogun | |||
| watch_method("some_method", &CMockObject::some_method); | |||
| } | |||
|
|
|||
| virtual CSGObject* create_empty() const | |||
There was a problem hiding this comment.
curios: why did you add this here
There was a problem hiding this comment.
Basically, since observe() emit a clone of the given value, we happens to call MockObject->clone(). However, the standard SGObject implementation of clone() assumes that there is a corresponding create() method, for that element, inside class_list.h. This is not true for MockObject since it is used only for testing purpose. Therefore, I needed to override create_empty such to return a new instance without relying on the class_list.h methods.
There was a problem hiding this comment.
ah yep, I remember that stuff now. thanks!
|
good! |
observe()method when called forSGVector/SGMatrix. The givenSGVector/SGMatrixwas not cloned correctly (since copying them just copy the pointer to the raw data and it does not make a clone of the data itself).I moved some useful type traits fromtype_case.htobase_types.hsince I did not want to pollute too muchSGObject.hwith unnecessary headers.Any comments on this change? @karlnapf @vigsterkr @gf712