JuliaAI / MLJFlow.jl

Connecting MLJ and MLFlow
MIT License
9 stars 0 forks source link

Need handling for models with zero hyperparameters #22

Closed ablaom closed 10 months ago

ablaom commented 1 year ago

ConstantClassifier is a model with no hyperparameters. If I change ConstantClassifer below to DecisionTreeClassifier, for example, then no error is thrown.

using MLJ
using .Threads
nthreads()
# 5

logger = MLFlowLogger("http://127.0.0.1:5000", experiment_name="rooster")
X, y = make_moons()
model = ConstantClassifier()
#model = (@load RandomForestClassifier pkg=DecisionTree)()

evaluate(
    model,
    X,
    y;
    logger,
)

# ERROR: HTTP.Exceptions.StatusError(400, "POST", "/api/2.0/mlflow/runs/log-parameter", HTTP.Messages.Response:
# """
# HTTP/1.1 400 Bad Request
# Server: gunicorn
# Date: Mon, 11 Sep 2023 19:09:40 GMT
# Connection: close
# Content-Type: application/json
# Content-Length: 163

# {"error_code": "INVALID_PARAMETER_VALUE", "message": "Missing value for required parameter 'key'. See the API docs for more information about request parameters."}""")
# Stacktrace:
#   [1] mlfpost(mlf::MLFlowClient.MLFlow, endpoint::String; kwargs::Base.Pairs{Symbol, String, Tuple{Symbol, Symbol, Symbol}, NamedTuple{(:run_id, :key, :value), Tuple{String, String, String}}})
#     @ MLFlowClient ~/.julia/packages/MLFlowClient/Szkbv/src/utils.jl:74
#   [2] mlfpost
#     @ ~/.julia/packages/MLFlowClient/Szkbv/src/utils.jl:66 [inlined]
#   [3] logparam(mlf::MLFlowClient.MLFlow, run_id::String, key::Symbol, value::ConstantClassifier)       
pebeto commented 1 year ago

@ablaom I just identified this problem in our code. The way we are identifying and returning leaf values is not correct. This issue is coming from MLJModelInterface.

isnotaleaf(::Any) = false
isnotaleaf(m::Model) = length(propertynames(m)) > 0

flat_params(m, ::Val{false}; prefix="") = NamedTuple{(Symbol(prefix),), Tuple{Any}}((m,))

When we pass the ConstantClassifier() (or any zero params model), isnotaleaf is returning false. From flat_params(...), our result is ( = ConstantClassifier(),). That's the source of our problem. The simplest way to solve this is to perform a validation, but maybe there is a better way to do that.

ablaom commented 1 year ago

@pebeto Thanks for the diagnosis. I've made the PR referenced above to give flat_params expected behaviour: flat_params(ConstantClassifier()) should return an empty tuple NamedTuple(). Can you please review?

ablaom commented 1 year ago

@pebeto That PR has been merged. How should we proceed?

pebeto commented 10 months ago

@ablaom, must we throw an error if the model contains zero parameters? Now we are logging the model without issues.

ablaom commented 10 months ago

I wouldn't throw an error if it is working as expected.