shogun
shogun copied to clipboard
shogun-toolbox
Replace Math::fequals with new utility function
We are in the process of dropping Math.h and one of the more commonly used operation there is fequals, which checks if the difference between two values is less than the type's epsilon. The task is to write this as a free function, probably in a new file in the util directory and drop Math::fequals.
@karlnapf @vigsterkr I am assuming we should also remove fequals_epsilon in env in favour of std::numeric_limits<T>::epsilon?
there are some issues: CMath is stateful and the fequals_epsilonis modified in some testing parts. I am not sure that is still the case (it was basically to decrease accuracy temporarily in the integration testing without having to pass the epsilon down through 10 functions). Given that serialization now works differently (and should be lossless?), the epsilon is never changed. In that case I agree
I've looked into math.h and the simplest solution is to lift the fequals function there and add it into a separate function/header file. Doing that will account for the stateful-ness of math.h without diving deep to ascertain the current status of the serialization (presumably lossless).
However, this seems way too simple necessitating my question- would that suffice as a solution? If not, could you please point out what/where this serialization is? @karlnapf @gf712
Hi! Is anyone working over this issue? I would like to take it up if nobody is. Thanks!
Go ahead. From the looks of it, I don't think anyone else is.
I'm stopped since it looked like a fair bit of work that might end up being unused if I end up in the wrong direction.
Thank you I am a newbie here. If possible could you please give me some direction/hints to help me get started and move forward with the task?
I'm not the best person for this. If you search for mathematics.h, you should be able to find the function that we are trying to replace. One potential solution is as I described/queried on the comment above.
@karlnapf @gf712. Hi, I am new in shogun and want to start contributing. Can I be assigned this issue ?
@karlnapf @gf712 can you please tell whether the other methods of Math.h (which are used by fequals function like abs<> ,fequals_abs, etc.) need to be moved to the same new file or to some other file or they should be kept in Math.h itself?
Hi, sure, those can be moved as well. Easiest is to start a PR and can take it from there.
@gf712 I have started a pr (#5130) . Not sure about why the tests are failing. Can you please guide me?
Hello im new here and i would like to start contributing. Is this thread still open/ not being worked on by anyone ? If so I would also try to get my fingers wet :)
