Replace obtain_from_generic with .as #4224
Conversation
|
Overlooking the diff, this looks good. |
|
Are these ALL instances of |
| @@ -43,23 +43,6 @@ class CGradientResult : public CEvaluationResult | |||
| */ | |||
| virtual const char* get_name() const { return "GradientResult"; } | |||
|
|
|||
| /** helper method used to specialize a base class instance | |||
There was a problem hiding this comment.
dont remove these methods for now (the examples might stop working)
| @@ -78,16 +78,4 @@ void CLatentFeatures::init() | |||
| { | |||
| SG_ADD((CSGObject**) &m_samples, "samples", "Array of examples", | |||
| MS_NOT_AVAILABLE); | |||
| } | |||
|
|
|||
| CLatentFeatures* CLatentFeatures::obtain_from_generic(CFeatures* base_feats) | |||
There was a problem hiding this comment.
as you can see, the current obtain_from_generic methods are inconsistent: some of the increase the reference counter, and some of them dont. This one doesnt.
as does NOT increase the reference counter. This explains all the segfaults in the CI.
Unfortunately, if you replace a obtain_from_generic that increased the reference counter, you will have to add a SG_REF after the call.
However, I actually think that neither as nor obtain_from_generic should increase the reference counter at all, so the best would be just to modifz the code downstream to not decrease the reference counter downstream
src/shogun/kernel/Chi2Kernel.cpp
Outdated
| } | ||
|
|
||
| /* since an additional reference is returned */ | ||
| SG_REF(kernel); |
There was a problem hiding this comment.
here is an example of an increased reference counter
| if (inference->get_inference_type()!=INF_FITC_REGRESSION) | ||
| SG_SERROR("Provided inference is not of type CFITCInferenceMethod!\n") | ||
|
|
||
| SG_REF(inference); |
There was a problem hiding this comment.
another increase. I think the ones that do the increase are the minority
syashakash
force-pushed
the
remove-obtain_from_generic
branch
2 times, most recently
from
2019210
to
a8a707f
Compare
|
I have not deleted any declarations and definitions of |
syashakash
force-pushed
the
remove-obtain_from_generic
branch
2 times, most recently
from
57cfb43
to
1171799
Compare
syashakash
force-pushed
the
remove-obtain_from_generic
branch
from
1171799
to
5def2ae
Compare
| @@ -120,10 +120,8 @@ CBinaryLabels* CGaussianProcessClassification::apply_binary( | |||
| if (m_method->get_inference_type()== INF_FITC_LAPLACE_SINGLE) | |||
| { | |||
| #ifdef USE_GPL_SHOGUN | |||
| CSingleFITCLaplaceInferenceMethod* fitc_method= | |||
| CSingleFITCLaplaceInferenceMethod::obtain_from_generic(m_method); | |||
There was a problem hiding this comment.
CSingleFITCLaplaceInferenceMethod::obtain_from_generic method increases the reference counter, but your replacement doesnt.
Do you see what I mean here? You have written that you have dealt with this, but that is not true here
There was a problem hiding this comment.
ah sorry, you in fact remove the UNREF below ...
all good!
|
Ok cool! thx for this! |
Refer #4191
This is partial wok. There are more cases that need to be re-factored.
PS The build is expected to fail terribly when interface options are set. This is because
build/src/interfaces/{language}/shogun{language}wrap.cxxneeds refactoring whereverobtain_from_genericis used.