Closed ablaom closed 5 months ago
Doesn't look like there's a significant difference.
Before this PR:
julia> @time_imports import MLJBase
413.2 ms MLJBase 23.21% compilation time
After this PR:
@time_imports import MLJBase
437.3 ms MLJBase 22.21% compilation time
(edited) This PR addresses a type instability for operations (
predict
,transform
, etc) acting on machines, as identified in #959 (although this PR does not resolve the particular issue there).I admit it is not clear to me that the performance gains here are likely to significantly benefit many use cases. But having done the work to identify these instabilities, I don't see harm in addressing them.
The type instability is not difficult to address in the case of machines attached to ordinary models, by annotating a currently abstract type in the
Machine
struct. However, in the special case of a machine attached to a symbolic model (which appear exclusively in learning networks), the type instability remains (and looks difficult to remove).Benchmarks
In 69 regression models, we compared the "high level"
predict(::Machine, ...)
method with the "low level"predict(::Model, ...)
method (edit plusreformat(::Model, ...)
) implemented by third party model providers. The benchmark code is hidden below:In the tables below:
Only models with
slow_down > 1.75
orbloat > 2
are reported.Before this PR
After this PR:
Note that machines serialised using #master cannot be deserialised after this PR. But I don't consider this triggers a breaking release.
To do: