Add BernoulliNB and Binarizer #306
Conversation
lib/scholar/naive_bayes/bernoulli.ex
Outdated
| x_binarize = | ||
| if opts[:binarize] != nil, | ||
| do: Scholar.Preprocessing.Binarizer.fit_transform(x, threshold: opts[:binarize]), | ||
| else: x |
There was a problem hiding this comment.
Try to move as much code as possible inside fit_n. For example, could this be moved there?
There was a problem hiding this comment.
after change error appeared on this line:
if opts[:binarize] != nil,
and i don't know how to fix it
(Protocol.UndefinedError) protocol Nx.LazyContainer not implemented for nil of type Atom.
There was a problem hiding this comment.
I believe opts[:binarize] != nil inside defn will want to treat those as tensors. Perhaps this may work:
case opts[:binarize] do
nil -> x
binarize ->Scholar.Preprocessing.Binarizer.fit_transform(x, threshold: binarize)
endIf it doesn't work we can leave it as is :)
There was a problem hiding this comment.
it worked, Thanks!
|
I would suggest removing the binarizer from I will performed a more detailed review somewhere this week. |
lib/scholar/naive_bayes/bernoulli.ex
Outdated
| |> Nx.new_axis(1) | ||
| |> Nx.broadcast(jll) | ||
|
|
There was a problem hiding this comment.
Is the explicit broadcast necessary?
|
|
||
| defnp binarize_n(tensor, opts) do | ||
| threshold = opts[:threshold] | ||
| Nx.select(Nx.greater(tensor, threshold), 1, 0) |
There was a problem hiding this comment.
| Nx.select(Nx.greater(tensor, threshold), 1, 0) | |
| tensor > threshold |
I can change it and report but I can't check what values were inserted so it will give some random numbers for not correct input instead of an error. Please let me know If you still want to remove this from fit. |
|
💚 💙 💜 💛 ❤️ |
I am adding BernoulliNB and tests. I needed a binarizer in it so I added it with tests to preprocessing.