Skip to content
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

Need handling for models with zero hyperparameters #22

Closed
ablaom opened this issue Sep 11, 2023 · 5 comments · Fixed by #33
Closed

Need handling for models with zero hyperparameters #22

ablaom opened this issue Sep 11, 2023 · 5 comments · Fixed by #33
Assignees
Labels
bug Something isn't working

Comments

@ablaom
Copy link
Member

ablaom commented Sep 11, 2023

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)       
@ablaom ablaom added the bug Something isn't working label Sep 11, 2023
@pebeto
Copy link
Member

pebeto commented Oct 13, 2023

@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
Copy link
Member Author

ablaom commented Oct 16, 2023

@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
Copy link
Member Author

ablaom commented Oct 27, 2023

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

@pebeto pebeto self-assigned this Jan 2, 2024
@pebeto pebeto linked a pull request Jan 2, 2024 that will close this issue
@pebeto
Copy link
Member

pebeto commented Jan 2, 2024

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

@ablaom
Copy link
Member Author

ablaom commented Jan 2, 2024

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

@pebeto pebeto closed this as completed in #33 Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants