search

IQVIA-ML / TreeParzen.jl

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

Should `linear_forgetting_weights` throw if `N == 0`? #17

Closed ghost closed 4 years ago

ghost commented 4 years ago

Describe the solution you'd like

The N == 0 is never triggered in our tests so the return case is not tested. https://github.com/IQVIA-ML/TreeParzen.jl/blob/532212cdf8b237d5214407e7c34569a60398b7b5/src/LinearForgettingWeights.jl#L8

What is the correct behaviour? Should it actually error? Why would N ever be 0 and what is the effect of returning an empty array?

yalwan-iqvia commented 4 years ago

I took a quick peek at the code it looks like all current cases will throw DimensionMismatch

yalwan-iqvia commented 4 years ago

^ Red herring

kainkad commented 4 years ago

I can't seem to find a case where N would be 0 (other than faking it). In fact even if N == 0 we don't need that check because the return value would be 0-element Array{Float64,1} which is the same as with this condition in place. The one that we definitely need is if N < lf so that we can avoid the negative length issue.