Sometimes the set of C.exp_manager["kwargs"]["uri"] will not take effect

[ChiahungTai]

Description

Sometime with R.uri_context(uri=uri_path): will not be worked.
After digging the problem. The yield self is not set uri to the temporaril uri.

Motivation and Context

Bug fix

How Has This Been Tested?

• Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
• If you are adding a new feature, test on your own test scripts.

Screenshots of Test Results (if appropriate):

1. Pipeline test:
2. Your own tests:

Types of changes

• Fix bugs
• Add new feature
• Update documentation

[ChiahungTai]

@you-n-g
Due to the MLFLow API change, the check failed and the message is "qlib/workflow/expm.py:428:15: E1101: Instance of 'MlflowClient' has no 'list_experiments' member (no-member)".
See
https://github.com/mlflow/mlflow/blob/1ee3816840509090e2a18338fcfba146d514ecfc/CHANGELOG.md#1290-2022-09-16 for more infomation.
[Tracking] Add mlflow_search_experiments API to R client and deprecate mlflow_list_experiments (#6576, @dbczumar)

Looks like we need to use mlflow_search_experiments to replace mlflow_list_experiments.
Do you have any suggestion about this issue?
The fastest solution is limit the mlflow to 1.30.0.

[ChiahungTai]

@you-n-g
I think I can use mlflow.client serach_experiments to support list_experiments.
In this way, you might need to force the mlflow to > 1.30.
So what is your opinion?

[you-n-g]

Hi, @ChiahungTai

I try to explain the design of the experiment system. It is a little complicated. Discussions are welcome if I fail to make it clear.

The ExpManager is expected to be a singleton and the global Config (e.g. C) is also a singleton (btw, we can have multiple Experiments with different uri).
So we try to align them together. So you can see that they share the same variable, which is called default uri

When the user starts an experiment, the user may want to set the uri to a specific uri (it will override default uri during this period), and then unset the specific uri and fallback to the default uri. _current_uri is that specific uri.
The uri shared by ExpManager and C is the default uri.

Originally, uri_context tries to temporarily set the default_uri. So the shared uri should be changed instead of _current_uri. (sometimes user doesn't start an experiment, so there is no _current_uri. but user want to query )

I think the problem is here. The _current_uri should not be set and uri should be ignored because of the shared mechanism between two singletons.

You can upload your issue as a test case like this. It will be helpful for other users.

[you-n-g]

Please ignore my reply.
I think the current design is not good enough. I'll try to create a new PR. You are welcome to review my new PR later.

You can upload your issue as a test case like this. It will be helpful for other users. Hope it would work right in my new PR.

Thanks.

[ChiahungTai]

@you-n-g
I would love to review your patch. I will try to write a test case for reproducation. I guess the problem is the missing of call ExpManager constructor too.
I can also help to review qlib/data part. I have done a class of DBStorage backend to support SQLite as qlib format backend in my own project.

BTW, I will reply #1326 after I finish a spirit in my own project. I can read/write Chinese. If something hard to explain in English, feel free to use Chinese.

[you-n-g]

@ChiahungTai
I'm excited about your great passion for Qlib.
I have invited you as a collaborator in Qllib. Hope that would make the collaboration smoother
众人拾柴火焰高. haha~

[you-n-g]

@ChiahungTai Please check if #1364 solves your problem and makes the logic easier.
Thanks

[you-n-g]

@ChiahungTai For the mlflow question.
I think it is OK to force the mlflow to > 1.30. in the future version of Qlib.
But the following things should be ensured

1. Make user all the mlflow-related API are well tested.
2. The mlflow > 1.30 is compatible with mlflow server and files based on older version.

Contributors are encouraged to help with the upgrade :)