-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add history_additions
to resolve #202 at least for grid searches
#205
Conversation
Thanks for this. The sample application is very helpful. This can probably be adapted for documentation. I think we are being too specific with the signature, and are unnecessarily ruling out some other possible use-cases. Rather than Otherwise, this looks good to me. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #205 +/- ##
==========================================
+ Coverage 86.44% 87.40% +0.96%
==========================================
Files 13 13
Lines 649 659 +10
==========================================
+ Hits 561 576 +15
+ Misses 88 83 -5 ☔ View full report in Codecov by Sentry. |
Or, we could just insist a history entry:
Thoughts @dpaetzel ? |
As in copy all properties of
Why would this be breaking? This seems to me to be no more breaking than the first proposal (where we add more than just a single property called |
(I'm with you on allowing more than just my use case.) |
yes.
Well, breaking if we remove the existing properties that will now become redundant. How about we go with this option but leave the redundant properties, and I'll add an issue to remove them in the next breaking release? |
Got it.
I think this is a sensible way to move forward. 👍 I'll update the PR in the next days (sorry for the delay!). |
5f4b83d
to
e2cbcd6
Compare
I undid the many small changes and only added the import MLJBase: recursive_getproperty
using MLJTuning
using MLJ
DTRegressor = @load DecisionTreeRegressor pkg = DecisionTree verbosity = 0
using DecisionTree: DecisionTree
N = 300
X, y = rand(N, 3), rand(N)
X = MLJ.table(X)
model = DTRegressor()
space = [
range(model, :max_depth; lower = 1, upper = 5),
range(model, :min_samples_split; lower = ceil(0.001 * N), upper = ceil(0.05 * N)),
]
struct TreeDepthSelection <: MLJTuning.SelectionHeuristic end
function MLJTuning.best(::TreeDepthSelection, history)
# Extract the depths of all folds of all history entries.
fparams = recursive_getproperty.(history, Ref(:(evaluation.fitted_params_per_fold)))
depths = [DecisionTree.depth.(getproperty.(fparam, :raw_tree)) for fparam in fparams]
# Compute the mean of the depths stored in `history_additions`.
scores = mean.(depths)
# Within this contrived example, the best hyperparametrization is the one
# resulting in the least mean depth.
index_best = argmin(scores)
return history[index_best]
end
function MLJTuning.supports_heuristic(::LatinHypercube, ::TreeDepthSelection)
return true
end
modelt = TunedModel(;
model = model,
resampling = CV(; nfolds = 3),
tuning = LatinHypercube(; gens = 30),
range = space,
measure = mae,
selection_heuristic = TreeDepthSelection(),
n = 5,
)
macht = machine(modelt, X, y)
MLJ.fit!(macht; verbosity = 1000)
display(report(macht).history[1].evaluation) |
This adds the
history_additions
option toTunedModel
which allows to specify a functionf
which is then called on each set of folds during tuning asf(model, fitted_params_per_fold)
wheremodel
is the configuration the tuner is currently looking at andfitted_params_per_fold
is the vector offitted_params(mach)
for eachmach
trained during resampling (e.g. this has 5 entries if 5-fold CV is used)—see here.This closes #202 to some extent since when using searches that do not adjust their search space based on their trajectory (e.g.
Grid
andLatinHypercube
), this allows to optimize with respect to functions that are not exclusively predictive performance–based. For example: