-
Notifications
You must be signed in to change notification settings - Fork 7
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
CompatHelper: bump compat for EvoTrees to 0.16 for package test, (keep existing compat) #93
CompatHelper: bump compat for EvoTrees to 0.16 for package test, (keep existing compat) #93
Conversation
…p existing compat)
47ba06f
to
7024765
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
=======================================
Coverage 96.65% 96.65%
=======================================
Files 11 11
Lines 867 867
=======================================
Hits 838 838
Misses 29 29 ☔ View full report in Codecov by Sentry. |
test/rstar.jl
Outdated
@@ -25,8 +25,8 @@ end | |||
@testset "samples input type: $wrapper" for wrapper in [Vector, Array, Tables.table] | |||
# In practice, probably you want to use EvoTreeClassifier with early stopping | |||
classifiers = ( | |||
EvoTreeClassifier(; nrounds=1_000, eta=0.1), | |||
Pipeline(EvoTreeClassifier(; nrounds=1_000, eta=0.1); operation=predict_mode), | |||
EvoTreeClassifier(; nrounds=1_000, eta=0.01), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it clear from the changelog why the parameters have to be changed? Or was the previous choice non-standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not from the changelog, but the failures seem to come from Evovest/EvoTrees.jl#250 changing defaults from (max_depth = 5, nbins=32)
to (max_depth=6, nbins=64)
. Reverting either of these to the former default causes the tests to pass again.
Looks like a lot of parameter combinations cause the tests to pass, but just barely. Perhaps a better approach is to increase the rtol.
This reverts commit 9d18cc0.
Pull Request Test Coverage Report for Build 6425341858
💛 - Coveralls |
BTW I hope with Evovest/EvoTrees.jl#259 we will soon be able to run tests without pulling in CUDA which should fix the test issue here: https://github.com/TuringLang/MCMCDiagnosticTools.jl/actions/runs/6418332322/job/17425902139?pr=93#step:6:361 |
This pull request changes the compat entry for the
EvoTrees
package from0.14.7, 0.15
to0.14.7, 0.15, 0.16
for package test.This keeps the compat entries for earlier versions.
Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.