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

Standardize yaml path definitions for the neps arguments #70

Open
danrgll opened this issue Apr 25, 2024 · 4 comments
Open

Standardize yaml path definitions for the neps arguments #70

danrgll opened this issue Apr 25, 2024 · 4 comments
Milestone

Comments

@danrgll
Copy link
Collaborator

danrgll commented Apr 25, 2024

Currently, neps.run() loads an optimizer with custom settings through two arguments: searcher_path and searcher. The current implementation uses searcher to specify the name of a YAML file within a folder (without including the .yaml extension) and searcher_path for the path to that folder.
This approach does not align with the YAML usage for other arguments like pipeline_space and run_args, which link directly to a YAML file.

Possible Solutions:

searcher: "/path/to/searcher_config.yaml"

or we remove this usage completely, because the same functionality of 'searcher_config.yaml' exist already in 'run_args'

@danrgll danrgll mentioned this issue Apr 25, 2024
12 tasks
@Neeratyoy
Copy link
Contributor

With the declarative interface, can we do,

searcher: "/path/to/searcher_config.yaml"
eta: 3

The equivalent call in Python code should be this right:

# `eta` is a searcher_kwarg in the yaml
neps.run(..., searcher=<path-to-searcher-config-yaml>, eta=3, ...)  

If so, then what is the blocker? If not, well, why not? 😅

@Neeratyoy
Copy link
Contributor

Neeratyoy commented Apr 28, 2024

@TarekAbouChakra and I chose that design when trying to manage the defaults for the searcher choice as strings
We believe the design can be improved overall, to be more reflective of the actual class definition default arguments, and also to be transparent for research usage
@TarekAbouChakra any thoughts/comments?
@eddiebergman could perhaps also chip in perhaps regarding the design of the searcher path to yaml and kwargs to overwrite it

@eddiebergman
Copy link
Contributor

I would have to see what the current state of things are to have a good opinion on this, I'd say get the easiest thing to work with the other PR first then come back to this

@danrgll
Copy link
Collaborator Author

danrgll commented Apr 29, 2024

@Neeratyoy @eddiebergman The current implementation for yaml follows the neps implementation:

For neps:
neps.run(searcher="custom_searcher_file_name", searcher_path="/path/to/folder", eta=3)

For the Declarative this transitions currently to this, because I didn't wanted to introduce two different behaviours:

run_args:
      searcher: "custom_searcher_file_name"
      searcher_path: "/path/to/folder"
      searcher_kwargs:
               eta: 3

@Neeratyoy I also thinking about something like this:

run_args:
        searcher: "/path/to/searcher_config.yaml"
        searcher_kwargs: # under this key I currently group all searcher arguments
                  eta: 3

and then align this for neps like you proposed.

@eddiebergman eddiebergman added this to the Declaritive milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants