facebookresearch / hydra

Hydra is a framework for elegantly configuring complex applications
https://hydra.cc
MIT License
8.55k stars 616 forks source link

[Bug/Question] `config_path` resolution is misleading #2274

Open stoney95 opened 2 years ago

stoney95 commented 2 years ago

🐛 Bug

Description

The config_path behaves differntly. It depends on if hydra.main is used in the executed script or it is part of a custom decorator located in another module. The issue is rather complex to describe - please take a look at the "To reproduce" section. I will describe the issue there in detail.

I actually want that the DictConfig provided by hydra.main is directly resolved to a dataclass that represents my config. I use hydra.utils.instantiate to achieve this. As I have multiple entry points ("main scripts") in my project that share the same config I want to move this step into a custom decorator that I can reuse in all entry points.

I also checked to find an other way to achieve the direct resolution, but could not find any. If there is a better solution to my problem, I will gratefully accept it as a solution.

Checklist

To reproduce

This issue has multiple cases. I created a small repo that displays each of these cases. I will also list the cases in the following and provide the corresponding information

1. Use @hydra.main directly in the script

This works as expected.

Note: I use a pathlib.Path object representing the absolute path to my conf folder as config_path.

2. Move hydra.utils.instantiate into decorator

In the next step I moved everything into a decorator which I can use instead of @hydra.main. This decorator is placed in the script itself. With this setup it works.

The decorator looks like this:

def ingest_config(func):
    @functools.wraps(func)
    @hydra.main(config_path=ROOT_DIR / "conf", config_name="config", version_base=None)
    def inner(dict_config):
        config = hydra.utils.instantiate(dict_config)
        func(config)

    return inner

Note: It is important that the decorator is placed in the script itself

3. Use custom decorator that's imported from a module

The next step is to move the decorator into a module as I would like to use it in multiple scripts. The module in this case is common.config. Then doing this the resolution of the config_path is not working anymore.

Note: To me it looks like hydra has problems handling the pathlib.Path. In the next section I cast the pathlib.Path to a str

4. Use custom decorator with str object representing the absolute path to conf folder as config_path

As the previous error looks like hydra is struggling with pathlib.Path as config_path I cast it to str. The path itself stays the same. This is not working. The error message changes.

Note: Hydra struggles to resolve the absolute path. The path hydra created from the absolute path is common..Users.simons.dev.hydra-reproduce-error.conf. The decorator where @hydra.main is used is located in src/common/config.py. Users.simons.dev.hydra-reproduce-error.conf is the location where the configuration is actually located.

5. Custom decorator with relative path pointing to the conf folder

The next step was to replace the absolute path with a relative path to the conf folder. This failed as well

Note: Hydra still struggles to find the conf folder. Eventhough the path is now resolved differently compared to what happend when using the absolute path

6. Custom decorator with relative path pointing to the conf folder inside the src directory

As a last step I copied the conf folder into src. I also renamed it to conf_inside_src to be sure that this folder is referenced. I used a custom decorator that points to this folder. This made it work!

Note: I don't understand why hydra works differently depending on the location of the conf folder. I also would like to keep conf outside the src folder.

Expected Behavior

I expect that the usage of an absolute path works. I would also expect that case 5 (Using a relative path to "conf" folder which is not in "src") works as well.

System information

Additional context

As stated in the beginning if there is an other way to achieve resolution of yaml files to dataclass objects, I would be happy to use it.

Jasha10 commented 2 years ago

Thanks very much for taking the time to organize these examples. I'll take a look...

stoney95 commented 2 years ago

@Jasha10 , thanks for taking a look. I prefixed the script names with the number of the cases from the issue. I hope this improves the "readability" of the examples.

stoney95 commented 2 years ago

@Jasha10 any updates?

Jasha10 commented 1 year ago

Apologies for the slow reply @stoney95. I'll get back to you on this asap.

omry commented 1 year ago

Why are you not using the compose API to create your config? hydra.main is definitely not designed to be used like this.

stoney95 commented 1 year ago

Is the compose API also providing the possibility to use overrides as runtime arguments? As far as I understand the compose API I would need to parse the overrides manually and provide them to compose.

Is there a reason why main should not be used in the way described above?