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

Investigate random forest crash when feature types are set #4054

Closed
karlnapf opened this issue Dec 27, 2017 · 9 comments
Closed

Investigate random forest crash when feature types are set #4054

karlnapf opened this issue Dec 27, 2017 · 9 comments

Comments

@karlnapf
Copy link
Member

Running a random forest regression without setting the feature type works fine (and produces correct results). The forest complains though: since the feature types are not set, it tells the user that it assumes continuous features:

shogun/src/shogun/multiclass/tree/CARTree.cpp line 273: Feature types are not specified. All features are considered as continuous in training

To get rid of the warning, the user sets the feature types to continuous (false)

rf.set_feature_types(np.zeros(N, dtype=np.bool))

which causes Shogun to die without an error message:

terminate called after throwing an instance of 'shogun::ShogunException'

I made a standalone notebook example here:
https://gist.github.com/karlnapf/47683ab0dd015a3e9c7ee60f05a2fec0

We want this to be fixed. In the likely case that this is a corner case of how the forest is set up, Shogun should at least give a proper error message and instruct the user what to do.

Good task for someone who wants to understand how the random forests in Shogun work. A bugfix is one of the most valuable contributions you can make. You will have to use a debugger to see what is going on inside Shogun's C++ code for this

This bug was reported recently in #4051

@vermashresth
Copy link
Contributor

vermashresth commented Dec 27, 2017

@karlnapf I would like to work on this

@vermashresth
Copy link
Contributor

vermashresth commented Jan 2, 2018

@karlnapf I looked at the code through debugger and found that it stops here . The feature length is 1 but we are providing feature_type array of length N so there is a mismatch. The code works fine with feature_type array of size 1
But it terminates without showing the error message.
I also looked into the code by @HunorSzabo in issue #4051 . The code terminates here because for training labels, the values are not set correctly in the loop and its length is still zero after assignment.
But I can't figure out why the REQUIRE and SG_ERROR statements are terminating without showing the specified error messages while SG_WARNING is working fine. Any idea where I should look into to resolve that?

@karlnapf
Copy link
Member Author

Thanks a ton for taking this up. The error message should be printed. Maybe it is Python? Did you manage to reproduce the same problem from c++?
Not sure what is going on, you have an idea how to fix the problem itself.

Approach:

  • Build unit test that fails
  • Fix the cause
  • unit test passes

@vermashresth
Copy link
Contributor

Hi @karlnapf
The c++ code in #4051 has init_shogun() without any arguments, thats why it skips printing
any messages. As for python I think the problem is in sg_print_functions.cpp. The print error function gets called but doesn't print anything. Everything works fine and all the error messages get printed if sg.init_shogun_with_defaults() is written at the start of the python code (and same for c++ code). But it should work fine without explicitly calling the default one.

Also, if I change PyErr_SetString(PyExc_RuntimeError, str) to PyErr_Warn(NULL, str) in the sg_global_print_error() function, just like in the warning function, it prints the error messages but as runtime warnings. So it's just that PyErr_SetString isn't printing the error and I am stuck at that. Any reason why that might be so?

@karlnapf
Copy link
Member Author

Yes, for c++, you want init_shogun_with_defaults.
For Python, that should be the default @lisitsyn @vigsterkr
@vermashresth thanks a lot for looking into this.

Now, leaving the error message printing aside, let's get down with this RF bug :)
Why is it that there is a size mismatch? Isn't there one feature type per feature?

@vermashresth
Copy link
Contributor

@karlnapf umm, isn't the size mismatch doing what it is supposed to do?

feats_train = sg.RealFeatures(X.reshape(-1,1).T)
print feats_train.get_num_features(), feats_train.get_num_vectors()

feats_test = sg.RealFeatures(X_test.reshape(-1,1).T)
print feats_test.get_num_features(), feats_test.get_num_vectors()

labels_train = sg.RegressionLabels(Y)
print labels_train.get_num_labels()

labels_test = sg.RegressionLabels(Y_test)
print labels_test.get_num_labels()
1 100
1 20
100
20

We have only 1 feature so there should be only 1 feature type too. Or am I missing something here?

@karlnapf
Copy link
Member Author

You are absolutely right, I don't know why I got confused here. So is there any problem after all? Doesn't seem like.

@vermashresth
Copy link
Contributor

Yup, no problem in random forest then. But isn't it troublesome for a user to not get error messages when input goes wrong? I think we should resolve the error printing part too. Can we change the call to init_shogun function in the swig file here to init_shogun_with_defaults ?

@karlnapf
Copy link
Member Author

@vigsterkr @lisitsyn what's you opinion on that?

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