search

stripe-archive / brushfire

Distributed decision tree ensemble learning in Scala
Other
391 stars 50 forks source link

Use partial order to support different split behaviors #87

Open erik-stripe opened 8 years ago

erik-stripe commented 8 years ago

This PR moves brushfire-core over to use spire.algebra.PartialOrder and spire.algebra.Order where applicable. At this point we don't actually use any partial orderings, but the support is there.

Before we merge this I'd like to add some tests to prove this is working, and maybe flesh out support for the is present use-case a bit. What do you all think we need?

Review by @tixxit and @avibryant.

tixxit commented 8 years ago

Tests are good, but I don't think we need to flesh out the is-present stuff in this PR. Just moving Dispatched over to using a PartialOrder would be enough.

erik-stripe commented 8 years ago

OK, that makes sense. I'll do that next.

erik-stripe commented 8 years ago

Alright, Dispatched is moved over (that was easy!).

erik-stripe commented 8 years ago

I updated this to master, should be able to be merged.

?r @tixxit @johnynek

johnynek commented 8 years ago

This looks good to me.

I do think if we are going to use spire more (and we should) we will get mileage out of the algebra work to make algebird, scalding and spire work better together.

That or give up and make a spire-algebird package that gives conversions between the various typeclasses.

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.