Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should put and get do implicit conversions? #5045

Open
gf712 opened this issue May 23, 2020 · 7 comments
Open

Should put and get do implicit conversions? #5045

gf712 opened this issue May 23, 2020 · 7 comments

Comments

@gf712
Copy link
Member

gf712 commented May 23, 2020

When we call put and get we expect the cast types to match exactly:

EXPECT_EQ(obj->get(Tag<int32_t>("int")), 10);
EXPECT_THROW(obj->get<float64_t>("int"), ShogunException);

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.

@karlnapf
Copy link
Member

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?

@gf712
Copy link
Member Author

gf712 commented May 28, 2020

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

@karlnapf
Copy link
Member

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

@gf712
Copy link
Member Author

gf712 commented May 28, 2020

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 gf712 changed the title Should put and get do implicit casting? Should put and get do implicit conversions? May 28, 2020
@karlnapf
Copy link
Member

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

@gf712
Copy link
Member Author

gf712 commented May 29, 2020

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...

@karlnapf
Copy link
Member

karlnapf commented Jun 1, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants