search

shogun-toolbox / shogun

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

remove num_lhs field in CKernel #4259

Open karlnapf opened 6 years ago

karlnapf commented 6 years ago

The field only causes problems. This is a refactoring exercise to remove the fiels, and instead query the underlying features.

Special cases might be CCustomKernel where no features are set. However, the base class should always query the underlying features.

The member function

virtual int32_t get_num_vec_lhs()
    {
    return num_lhs;
}

would be changed to

virtual int32_t get_num_vec_lhs()
{
    return lhs->get_num_vectors();
}

or similar. Obsiously, the same should be for num_rhs

vishal3410 commented 5 years ago

I would like to work on this issue, Does it involve changing anything else other than line 514 and 523 in kernel.h and should i replace them as same as you described above and send a pr ?

karlnapf commented 5 years ago

it involves doing these changes and then checking whether they break anything. This involves reading the code, thinking, and running the tests. Please check here how to get started: https://github.com/shogun-toolbox/shogun/wiki/Getting-involved

divyanshu132 commented 5 years ago

@karlnapf I'm not able to perform all the tests, can you please help me out!!

vishal3410 commented 5 years ago

Hello @divyanshu132 I am working on this issue if u don't mind can you please leave this issue to me ?, i will let u know if i am not able to fix this

divyanshu132 commented 5 years ago

@vishal3410 no problem, it's completely fine!!