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

XGBoostJsonParser not working well with 'binary features' #152

Open
xwang-saj opened this issue Apr 19, 2018 · 4 comments
Open

XGBoostJsonParser not working well with 'binary features' #152

xwang-saj opened this issue Apr 19, 2018 · 4 comments

Comments

@xwang-saj
Copy link

xwang-saj commented Apr 19, 2018

The current setup of the plugin requires a feature map to be used for creating serialized xgboost json model file (for an example of feature map see this).

In the feature map, each feature can be assigned 3 possible data types: q (quantitate), i (binary) and int (integer).

When the data type is int or q, each split node will be serialized to look like below:

      { "nodeid": 6, "depth": 2, "split": "f1", "split_condition": 5, "yes": 13, "no": 14, "missing": 14, "children": [
        { "nodeid": 13, "leaf": 0.000920585 },
        { "nodeid": 14, "leaf": -0.044742 }
      ]}

However, when data type is i, each split node would look like this after serialization:

 { "nodeid": 4, "depth": 2, "split": "f2", "yes": 9, "no": 10, "children": [
        { "nodeid": 9, "leaf": 0.138548 },
        { "nodeid": 10, "leaf": -0.0143873 }
      ]}

Basically, there will be no field for 'missing' and 'split_condition'.

The current XGBoostJsonParser though, explicitely checks for existence of split conditions and therefore throws exceptions when parsing binary nodes. (The code below is copied from here:)

boolean splitHasAllFields() {
            return nodeId != null && threshold != null && split != null && leftNodeId != null && rightNodeId != null && depth != null
                    && children != null && children.size() == 2;
  }

What I suggest for the fix:

  1. In the short term, define all binary features into integer features and notify users of this limitation somewhere in the documentation.
  2. In the long run, revise splitHasAllFields() to account for the data type of the split nodes, or just eliminate the check on split conditions and threshold, or provide default values for binary split nodes.
@aditya-malte
Copy link

@shah-sid-cutshort are you facing this same problem?

@aditya-malte
Copy link

Hey @o19s-admin,
This seems to be an old issue, do we have an update/fix on it? If not, then it must be at least mentioned somewhere in the docs that boolean may not be supported and the training script must take this into consideration.

@lonngxiang
Copy link

how to serialized xgboost json format like that? the link is broken now

@nathancday
Copy link
Member

Is this the demo you are looking for?
http://es-learn-to-rank.labs.o19s.com/

Branches for older versions of elastic are kept, so if you ever need "older" material that is a good way to find it. Aside from that we want to improve the documetation around xgboost models and would appreciate any help from the community

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

4 participants