Skip to content
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

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

Closed
svilupp opened this issue Apr 21, 2023 · 4 comments · Fixed by #223
Closed

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

svilupp opened this issue Apr 21, 2023 · 4 comments · Fixed by #223

Comments

@svilupp
Copy link
Contributor

svilupp commented Apr 21, 2023

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:

  • Once features are split, all data points are bucketed into corresponding slots by function binarize
  • This line makes an assumption that the buckets are of type UInt8, so implicitly assuming there are never more than 255 bins

Suggested solution:

  • 255 bins should be sufficient and no user has run into this limitation so far, so it can be kept
  • we should add a note in the documentation
  • and check this value on initialization (with a helpful error message)

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

@svilupp
Copy link
Contributor Author

svilupp commented Apr 21, 2023

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
Copy link
Member

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
Copy link
Contributor Author

svilupp commented Apr 22, 2023

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
Copy link
Contributor Author

svilupp commented Apr 23, 2023

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

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 a pull request may close this issue.

2 participants