shogun-toolbox / shogun

Shōgun
http://shogun-toolbox.org
BSD 3-Clause "New" or "Revised" License
3.03k stars 1.04k forks source link

cleanup memory allocator/deallocator inconsistencies #4871

Open vigsterkr opened 4 years ago

vigsterkr commented 4 years ago

In many cases in shogun there are still plain SG_MALLOC/CALLOC calls that allocates a memory region then that region is either passed to SGVector or SGMatrix but not only to wrap that region for interface conformity but as well to pass that memory region to be managed by SGMatrix/SGVector... i.e. when you see something like this:

float64_t mem = SG_MALLOC(float64_t, size);
....
SGVector<float64_t> v(mem, size);

in this case the SGVector will call a deallocator on the mem region, but it's actually not correct as SGVector might call a wrong type of deallocator (incompatible with the SG_MALLOC one).

the other case when you see something like this

float64_t mem = SG_MALLOC(float64_t, size);
....
SGVector<float64_t> v(mem, size, false);

is totally valid as in this case the mem management is not being handed over...

here's a patch that actually solves a problem like this: https://github.com/shogun-toolbox/shogun/pull/4719/commits/edf6f4d2783fd9c870a06ddc94ee9fdd87d196f2

in order to find these cases the best course is to actually do:

cd src/shogun
git grep SG_MALLOC

and check what are those malloc are actually doing.

this is required to be fixed all along the whole library in order to get the windows build working properly....

tanaymeh commented 4 years ago

I would like to fix a few of these throughout the codebase. I will get back with PR as soon as I have done some work!

vigsterkr commented 4 years ago

currently failing tests are: KMeans.minibatch_fixed_centers MeanRule.combine_matrix MalsarTraceTest.train MalsarClusteredTest.train MalsarL12Test.train NormOneTest.apply_to_vector RandomForestTest.score_compare_sklearn_toydata CARTree.comparable_with_sklearn MulticlassLibLinearTest.train_and_apply BaggingMachineTest.output_binary Math.linspace_test SGSparseMatrix.io_libsvm ROCEvaluation.one

these will definitely have somewhere such a case mentioned above

Aakash1822 commented 4 years ago

Interested to work on it @vigsterkr. Please guide me

vigsterkr commented 4 years ago

@Aakash1822 i've given example in the description and there's the list of the errors failing. you need to find similar (like in the example) the classes that are being used in those tests