IQVIA-ML / TreeParzen.jl

TreeParzen.jl, a pure Julia hyperparameter optimiser with MLJ integration
Other
35 stars 5 forks source link

Brittle method for generating new MLJ model instances with parameter changes #95

Closed ablaom closed 1 year ago

ablaom commented 1 year ago

Describe the bug The bug is reported and explained here.

To Reproduce Steps to reproduce the behavior: See the post above.

Expected behavior No fail.

Environment (please complete the following information): Unknown, but likely not relevant given the diagnosis already given.

Additional context Here's a further clarification of the issue:

julia> struct Bar{T}
       x::T
       end

julia> Bar(; x=1)=Bar(x)
Bar

julia> Bar(x=5)
Bar{Int64}(5)

julia> Bar{Int64}(x=5)
ERROR: MethodError: no method matching Bar{Int64}(; x=5)
Closest candidates are:
  Bar{T}(::Any) where T at REPL[4]:2 got unsupported keyword argument "x"
Stacktrace:
 [1] top-level scope
   @ REPL[7]:1

Instead of constructing new model instances with typeof(mode)(; parameters), which is failing for model types with parameters as above, I suggest creating a deep copy of the model and mutating the parameters using setproperty for each property in propertynames(model).

kainkad commented 1 year ago

Thank you @ablaom for raising this issue and apologies for a delayed response. I have submitted a PR #96 which is now mutating the model parameters for each candidate (trial) rather than constructing new model instances based on their types. I have tested it with latest EvoTrees (both classifier and regressors) as well as with a selection of other MLJ supported models. Once the PR is reviewed and merged, we'll get it released and notify you.

kainkad commented 1 year ago

The fix is now available on v0.3.3

ablaom commented 1 year ago

Thank you @kainkad. Appreciate the ongoing support of this package.