Skip to content

Turn off supportSparse in GitHubLabeler sample #257

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

Merged
merged 1 commit into from
Feb 16, 2019

Conversation

stephentoub
Copy link
Member

The input isn't sparse, but the ReadFromTextFile is using the default of
supportSparse=true, which is causing the loader to try and fail to parse
every row as if it were sparse (you can see this if you hook up an
mlcontext.Log handler and print out every message containing "Format").

Related to dotnet/machinelearning#2576
cc: @CESARDELATORRE, @eerhardt

The input isn't sparse, but the ReadFromTextFile is using the default of
supportSparse=true, which is causing the loader to try and fail to parse
every row as if it were sparse (you can see this if you hook up an
mlcontext.Log handler and print out every message containing "Format").
@CESARDELATORRE
Copy link
Contributor

CESARDELATORRE commented Feb 16, 2019

Questions:

  • Is this improving the performance (training time)?
  • If this case that we need to provide "supportSparse: false", the most common case? - My point is, should this parameter be false for most of the samples? if yes, why is supportSparse not defaulted to false in the API?

I find this parameter to be "yet another" concept in the "by default" API...

In any case, I'm going to take the PR since with current API it is the best approach. 👍

@CESARDELATORRE CESARDELATORRE merged commit d884cb4 into dotnet:master Feb 16, 2019
@stephentoub stephentoub deleted the disablesparse branch February 17, 2019 17:07
stephentoub added a commit that referenced this pull request Mar 5, 2019
CESARDELATORRE pushed a commit that referenced this pull request Mar 5, 2019
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 this pull request may close these issues.

3 participants