-
Notifications
You must be signed in to change notification settings - Fork 29
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
Improvements to support naive Bayes classification of text documents #401
Comments
Yes, I am definitely interested. I am in the middle of the final push of the Julia Data Science 1st edition final chapter. I cannot help with authoring PRs now. Maybe in 1 month I will have time. But I can give insight and help review some PRs. One thing that might be interested to discuss is |
I'd definitely like to support this! I agree the changes we'd need to make the existing interface would be pretty minimal. I can take a look at it. |
@storopoli I guess algorithms that use dictionaries as read-only should be fine. So supplying dictionaries to NaiveBayes.jl should be okay. However, I see that the TextAnalysis version does write to dictionaries, and it seems the dictionaries are used as a way to allow for incremental training (BTW, not yet supported by MLJ). I'm not familiar enough with NLP to know how important this feature is. But it's unlikely this implementation would be thread-safe. @pazzo83 Be great if you could give NaiveBayes.jl some TLC. The author is not actively maintaining but has been quick to review outside PR's and I have some sort of write access there too, I think. Adding sparse array support is low hanging fruit. But perhaps a quick review to determine if a complete re-design is the better path, if that's something you can assess. |
Currently we wrap NaiveBayes.jl, but only allow tabular input (internally converted to matrix) which limits application to NLP and elsewhere. However NaiveBayes.jl itself supports dictionary input.
There is also a text-specific NaiveBayes classifier in TextAnalysis, which accepts dictionaries (keyed on abstract strings).
I have never used either package seriously but my gut feeling is that there would be little difference in performance when using dictionaries, and would suggest we simply enhance the existing interface at MLJNaiveBayesInterface.jl rather than write a new interface. Also, there would be no reason I see to restrict to abstract string keys - any abstract dictionary with
Integer
values should be supportable. Such objects have the scientific typeMultiset{S}
whereS
is the scitype of the keys. So we could support anyMultiset
as input.Another possibility might be to add support for sparse matrices, probably adjoints of julia's
SparseArrayCSC
matrices (input to MLJ model is n x p by convention). However this requires a small generalisation of the NaiveBayes package (needed anyway) which at the moment only supports concreteMatrix
types.Very happy to hear some different suggestions for improving naive bayes support.
@storopoli Perhaps this is something you would be interested in helping out with?
cc @pazzo83
The text was updated successfully, but these errors were encountered: