register factory methods for proper garbage collection #4322
Conversation
src/interfaces/swig/factory.i
Outdated
| %newobject ecoc_encoder(const std::string& name); | ||
| %newobject ecoc_decoder(const std::string& name); | ||
| %newobject features(SGMatrix<T> mat); | ||
| %newobject features(CFile* file, EPrimitiveType primitive_type = PT_FLOAT64); |
There was a problem hiding this comment.
I am not sure whether default arguments should be here
src/interfaces/swig/factory.i
Outdated
| %newobject distance(const std::string& name); | ||
| %newobject evaluation(const std::string& name); | ||
| %newobject kernel(const std::string& name); | ||
| %newobject kernel(SGMatrix<T> kernel_matrix); |
There was a problem hiding this comment.
@shubham808 did you check whether shogun has any other %newobject cases where a templated object is returned? I wonder whether this needs some special treatment (and idk)
There was a problem hiding this comment.
mmh... i thought that too but did not find any @vigsterkr any ideas ?
There was a problem hiding this comment.
as far as SWIG goes it doesn't understand templates, before they are actually instantiated, say
%template SGMatrix<float32_t>
%template SGMatrix<float64_t>
that's the general guideline of SWIG... dunno how newobject works in this context but i would expect the same (consistent) behaviour?
There was a problem hiding this comment.
just checked, and there is no case of %newobject in shogun that is templated.
@shubham808 i think let's just do it for float64 for now ...
shubham808
force-pushed
the
feature/garbage_collection_factory_methods
branch
from
830b189
to
eaa368e
Compare
src/interfaces/swig/factory.i
Outdated
| %newobject ecoc_encoder(const std::string& name); | ||
| %newobject ecoc_decoder(const std::string& name); | ||
| %newobject features(SGMatrix<T> mat); | ||
| %newobject features(CFile* file, EPrimitiveType primitive_type); |
There was a problem hiding this comment.
in fact we dont even need the parameter names, just the types
shubham808
force-pushed
the
feature/garbage_collection_factory_methods
branch
from
eaa368e
to
895d7da
Compare
src/interfaces/swig/factory.i
Outdated
| %newobject multiclass_strategy(const std::string&); | ||
| %newobject ecoc_encoder(const std::string&); | ||
| %newobject ecoc_decoder(const std::string&); | ||
| %newobject features(SGMatrix<T>); |
|
Ok looks good now. Now what does the garbage collector say in SWIG interfaces? works? |
|
i am not sure... should i run all examples with valgrind ? |
|
It is pretty simple to find this out using valgrind ... |
|
valgrind says 0 leaks for all meta examples in python |
|
Have u made sure that all the hacks are being in place for memleak detection of python tool with valgrind, eg https://stackoverflow.com/a/45248015
… On 4 Jun 2018, at 16:17, Shubham Shukla ***@***.***> wrote:
valgrind says 0 leaks for all meta examples in python
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Key is the delta of this before and after patch |
|
i used the gcdebug log level in a python script that overwrites a kernel generated from factory. |
|
Yeah ok, we will need to dig into this a bit I guess.....I wonder whether it is the SG_REF thingi in the factory that causes it.... |
|
I leave this open for now as we should fix all the memory leaks in the examples and the factory at some point |
|
"superseded" by #4604 |
#4321