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 support for muitl-variable replacement for PipelineController.add_step() parameter_override's. #1089

Open
natephysics opened this issue Aug 4, 2023 · 4 comments

Comments

@natephysics
Copy link
Contributor

natephysics commented Aug 4, 2023

Proposal Summary

Tested with pipelines from tasks.

When adding a step to a pipeline it's possible to override parameters. When overriding parameters, it's possible to do variable replacement of the parameters for parameters from the prior stages of the pipeline, e.g. parameter_override={'Args/input_file': '${stage3.id}' }. However, this feature doesn't appear to currently support overriding multiple parameters in the same argument, e.g. parameter_override={'Args/input_file': '[${stage3_task_A.id}, '${stage3_task_B.id}', '${stage3_task_C.id}']}. There are a lot of potential use cases for such a feature.

It is possible to do something like this using a dedicated argument for each variable being replaced. But that method would then create a huge number of potential variables that would need to be discovered and iterated over when it would be much simpler to have a single argument with a list of values.

This should be a fairly non-intrusive change to the codebase. I'd happily create a PR for it if someone would be willing to direct me to the relevant parts of the code (I've looked with the help of someone on Slack but it wasn't obvious).

Motivation

I have a pipeline that has a variable number of parent stages that will feed into one child. It would be much easier to return a list and iterate over the list rather than discover each variable.

Related Discussion

https://clearml.slack.com/archives/CTK20V944/p1689609314325429

@AlexandruBurlacu
Copy link

Hey @natephysics, we always appreciate any contributions, and I'll be glad to help you! That being said, I believe to support your suggestion, you'd need to check the _parse_step_ref method at clearml/automation/controller.py:2779 and extend it's functionality to handle multiple references per parameter

@natephysics
Copy link
Contributor Author

Sorry, it took me a while to put it together. I was on vacation.

#1099

@pollfly
Copy link
Contributor

pollfly commented Oct 2, 2023

Hey @natephysics! v1.13.0 is now out, supporting recursive list, dict, and tuple ref parsing for PipelineController.add step() parameter overrides

@natephysics
Copy link
Contributor Author

Hey @natephysics! v1.13.0 is now out, supporting recursive list, dict, and tuple ref parsing for PipelineController.add step() parameter overrides

Awesome! I'm glad it was included.

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