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

Java Wrapper H2OUtils doesn't check type in NDArray proto message conversion #158

Closed
ukclivecox opened this issue Jun 1, 2018 · 1 comment · Fixed by #161
Closed

Java Wrapper H2OUtils doesn't check type in NDArray proto message conversion #158

ukclivecox opened this issue Jun 1, 2018 · 1 comment · Fixed by #161

Comments

@ukclivecox
Copy link
Contributor

https://github.com/SeldonIO/seldon-core/blob/master/wrappers/s2i/java/wrapper/src/main/java/io/seldon/wrapper/utils/H2OUtils.java#L71

We should check the type of the items and convert to appropriate type if possible.

@ukclivecox
Copy link
Contributor Author

Looking at the H2O docs for RowData:

Columns of different types are handled as follows:
For numerical columns, the value Object may either be a Double or a String. If a String is passed, then Double.parseDouble() will be called on the String.
For categorical (aka factor, enum) columns, the value Object must be a String with the same names as seen in the training data. It is not allowed to use new categorical (aka factor, enum) levels unseen during training (this will result in a hex.genmodel.easy.exception.PredictUnknownCategoricalLevelException when one of the predict methods is called).

We can update the wrappers to create Doubles or Strings depending on what is in the NDArray ListValue, but the above docs suggest you should send categorical data as Strings.

agrski pushed a commit that referenced this issue Dec 2, 2022
* pipeline inputs

* clean notebook

* fix tests

* review comments

* fix test

* remove comment

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

Successfully merging a pull request may close this issue.

1 participant