Evovest / EvoTrees.jl

Boosted trees in Julia
https://evovest.github.io/EvoTrees.jl/dev/
Apache License 2.0
175 stars 21 forks source link

InexactError: trunc(UInt8, 256) when user sets nbins>255 #222

Closed svilupp closed 1 year ago

svilupp commented 1 year ago

Current behaviour: If a user sets nbins>255 they will get InexactError: trunc(UInt8, 256)

Expected behaviour: Either the restriction should be explicit and caught on initialization or the code should be updated.

Possible cause:

Suggested solution:

I'm happy to take a stab at the PR if helpful.

svilupp commented 1 year ago

MWE (taken from the example in docs):

using EvoTrees
using Random

x_train = randn(100, 10)
y_train = randn(100)

config = EvoTreeRegressor(
    loss=:linear, 
    nrounds=100, 
    max_depth=6, 
    nbins=256,
    eta=0.1,
    lambda=0.1, 
    gamma=0.1, 
    min_weight=1.0,
    rowsample=0.5, 
    colsample=0.8)

m = fit_evotree(config; x_train, y_train)
preds = m(x_train)

# throws: nested task error: InexactError: trunc(UInt8, 256)

Tested on the latest version in main. versioninfo()

Julia Version 1.8.5 Commit 17cfb8e65ea (2023-01-08 06:45 UTC) Platform Info: OS: macOS (arm64-apple-darwin21.5.0) CPU: 8 × Apple M1 Pro WORD_SIZE: 64 LIBM: libopenlibm LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1) Threads: 6 on 6 virtual cores

jeremiedb commented 1 year ago

Adding domain validation of the arguments passed to the hyper-params constructors is something I've been shoveling forward a bit too long! Thanks for reporting, feel free to go for a PR, otherwise, I'll likely get it done with 1-2 days.

svilupp commented 1 year ago

I have too many other things going on this weekend, but I should be able to get to it on Monday.

When I find time, I'll check here if you've started or not.

svilupp commented 1 year ago

I've hacked up a draft. Let me know what you think!