shogun icon indicating copy to clipboard operation
shogun copied to clipboard

Should put and get do implicit conversions?

Open gf712 opened this issue 4 years ago • 7 comments

When we call put and get we expect the cast types to match exactly: https://github.com/shogun-toolbox/shogun/blob/93a649bb9bacfd342875eae40115d37e74d1f0f8/tests/unit/base/SGObject_unittest.cc#L486-L487 However, does this make sense? In C++ it is valid for me to call a function with an implicit conversion:

void f(double a);
int main()
{
    f(1);
}

Similarly, it is valid to implicitly cast a return type (so a getter can return int, but we store it as float64_t). So the test from above would fail, because the types would be casted implicitly, if we were using a getter (rather than SGObject::get):

float64_t data = get_int(); // no compiler error

From a technical perspective this is possible to implement. My question is: would it bring more bugs than it would avoid? For example, on one hand a user might lose information by casting float to int, but on the other the program won't crash after 10 hours of training because of the type mismatch. I would also not want shogun to become like JS where everything has some result because the type system is too flexible.

gf712 avatar May 23 '20 09:05 gf712

I'd prefer if we had a more relaxed typing with the parameter framework. Then we also wouldnt have to do these horrendeous workarounds in swig that I added to avoid problems with this. E.g. in java everything is a float64, in octave matrices with one element automatically get converted to scalars, 1.0 gets converted to int, etc. I'd basically expect the same implicit casting as C++ compilers do?

karlnapf avatar May 27 '20 21:05 karlnapf

I guess that means every arithmetic type can be casted to any other arithmetic type?

gf712 avatar May 28 '20 15:05 gf712

what happens in c if you do int a = 3.4?

karlnapf avatar May 28 '20 20:05 karlnapf

It depends. With -Wconversion you get a warning and I guess if you combined that with -Werror you get a compiler error. Without that warning the floating point type is floored, ie a=3

gf712 avatar May 28 '20 21:05 gf712

I see....mmmh. Probably we would want to follow the behaviour that we have in our build, i.e. without those flags?

karlnapf avatar May 29 '20 16:05 karlnapf

Yes, I agree. I think that at least all integer types should be implicitly converted between them using the safe conversion helpers we already have. The same for floating points. Just from int to float I am not sure about...

gf712 avatar May 29 '20 16:05 gf712

What I would do then is to have everything that doesnt loose information automatically. And then the ones which might loose information can simply print a runtime warning from shogun

karlnapf avatar Jun 01 '20 09:06 karlnapf