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

Fix Julia category mapping #3305

Merged
merged 3 commits into from Nov 7, 2022

Conversation

rcurtin
Copy link
Member

@rcurtin rcurtin commented Nov 3, 2022

Julia is a one-indexed language; so, when the user gives us labels from Julia, we expect them to be in the range [1, numClasses]. Internally, we must map them to [0, numClasses - 1] when passing to mlpack, since mlpack and Armadillo are zero-indexed and that is our convention. For labels, this works just fine.

When we pass categorical data, we have the same one-indexed expectation from Julia: the Julia user will give us a Matrix{Float64} as well as a Vector{Bool}. The boolean vector specifies which dimensions are categorical, and then those dimensions in the Matrix{Float64} should take values in [1, numCategories]. Here, we must also do the same transformation to [0, numCategories - 1].

However, that transformation was not implemented, which caused the failure in #3300.

This PR fixes the problem, by subtracting one from any categorical dimensions before passing the data into mlpack.

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit bacaa23 into mlpack:master Nov 7, 2022
@rcurtin rcurtin deleted the julia-fix-categorical-mapping branch November 7, 2022 15:35
@rcurtin rcurtin mentioned this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants