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

Add inference parameters to KedroPipelineModel #445

Open
Galileo-Galilei opened this issue Aug 18, 2023 · 3 comments · May be fixed by #580 or #612
Open

Add inference parameters to KedroPipelineModel #445

Galileo-Galilei opened this issue Aug 18, 2023 · 3 comments · May be fixed by #580 or #612
Assignees
Labels
enhancement New feature or request

Comments

@Galileo-Galilei
Copy link
Owner

Galileo-Galilei commented Aug 18, 2023

Description

Mlflow introduced inference parameters in 2.6.0.

Context

Passing parameters at inference time to avoid retraining for long applications (e.g. probability threshold, LLM temperature...) is a long demanded feature

Possible Implementation

Update PipelineML and MlflowHook to propagate this argument

@Galileo-Galilei Galileo-Galilei added enhancement New feature or request need-design-decision Several ways of implementation are possible and one must be chosen labels Oct 25, 2023
@Galileo-Galilei Galileo-Galilei moved this from 🆕 New to 🔖 Ready in kedro-mlflow roadmap Oct 28, 2023
@Galileo-Galilei
Copy link
Owner Author

Open question: which parameters can be passed at runtime?

  • all parameters ?
  • Only the ones marked witht the "runtime_params" resolver?
  • Only the ones marked witht the "globals" resolver?
  • Only the ones with a specific resolver?

@Calychas Calychas linked a pull request Aug 13, 2024 that will close this issue
6 tasks
@Galileo-Galilei
Copy link
Owner Author

I'd like to go with a two step implementation because there are 2 part of the workflow modified here:

  • the "training time", i.e. the time where we call kedro run -p training on a pipelineML object. We want to propagate default arguments for KedroPipelineModel.predict from pipeline_ml_factory to the KedroPipelineModel. This exists partially (e.g the runner argument can already be propagated all way long (see Enable asynchronous mode when serving inference pipeline #587 (comment))
  • the "inference time" i.e. the time where we call KedroPipelineModel.predict. The key advantage to modify non training related parameters / running configuration through predict is that you decouple this configuration from training. You should never be forced to retrain a model just to predict with a different runner.

Step 1 : Define the interface we want to pass to the KedroPipelineModel.predict() at inference time

There are 3 objects we want to be able to modify at runtime, hence we need to pass them all to the params argument:

I suggest we take over the PR #580 to experiment on this. The PR should only modify the predict function (no propagation / impact on pipeline_ml_factory and KedroPipelineModel.

I roughly imagine something like this :

  • params is a dictionary with several keys {"runner": ..., "hooks": ..., "parameters":...}
  • we retrieve and validate each key, then define the logic which sould look a lot like KedroSession._run.
  • There are some things we should test and clarify, but we can do it directly in the PR:
    • how do we pass instantion param to each object?
    • do we pass class? strings? instances of classes (especially for hooks)?
    • should we support all the possibility of KedroSession.run? WIP: enabling hooks for KedroPipelineModel serving #608 suggest not implementing after_context_created and after_catalog_created
  • I don't know how it is technically possible to render runtime_params, because KedroPipelineModel is only aware of the resolved catalog and not the raw configuration and I am afraid we will be forced to only override parameters and not runtime_params or globals. This will hopefully change with the new catalog redesign (Design DataCatalog2.0 kedro-org/kedro#3995).
  • I think it would be good to add a method to list overridable parameters for readibility, e.g. KedroPipelineModel.parameters should return dynamically paramters from the stored catalog.

Step 2 : Once the KedroPipelineModel.predict() interface is well defined, propagate default from pipeline_ml_factory

It would me mostly taking over PR #608 to propagate default (add predict_params or something like that in pipeline_ml_factory, and in KedroPipelineModel). Notice that as of now, I am more in favor or removing predict params (like runner) from pipeline_ml_factory than adding new ones. This should be assessed after step 1 is merged and well tested.

@Calychas @Lasica are you fine with this? @Calychas should we work simultanesouly on your branch? If it is simpler for both of you, do you want developer access to kedro-mlflow instead of the fork?

@Galileo-Galilei
Copy link
Owner Author

I've started to POC this, and there are two bad news about this feature:

  1. Mlflow predict only support params defined in the signature at logging time : https://mlflow.org/docs/latest/model/signatures.html#signature-enforcement
  2. There is type enforcement, so we cannot pass arbitrary objects (e.g. hooks): https://mlflow.org/docs/latest/model/signatures.html#supported-data-types-for-params

Passing runner and parameters should be fairly easy, but unfortunately I don't think we can pass arbitrary hooks this way @Calychas.

@Galileo-Galilei Galileo-Galilei removed the need-design-decision Several ways of implementation are possible and one must be chosen label Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🏗 In progress
1 participant