opendatahub-io / odh-dashboard

Dashboard for ODH
Apache License 2.0
28 stars 160 forks source link

[Feature Request]: surfacing nested pipeline input parameters #1519

Closed yuanchi2807 closed 9 months ago

yuanchi2807 commented 1 year ago

Feature description

Components participating in a nested pipeline have their own custom configurations. For example, in one component, an English language model is loaded to process input data while in another component, it is configured to load a Spanish language model.

A flat layout of all rolled up configuration parameters from all components can be challenging to interpret by pipeline users. It may be worth considering to improve user experience with grouping or hierarchical layouts.

Describe alternatives you've considered

No response

Anything else?

No response

andrewballantyne commented 1 year ago

@yuanchi2807 When you refer to nested Pipeline input parameter... how "nested" are we talking here? The way I understand how Tekton works is:

Here is the Tekton variable reference doc: https://tekton.dev/docs/pipelines/variables/

Could you provide more information?

yuanchi2807 commented 1 year ago

@andrewballantyne Thanks for the link. On the DSP 'create run' configuration screen, how should I expect pipeline and task parameters be organized under sections?

andrewballantyne commented 1 year ago

@yuanchi2807 Creating a new Pipeline Run only has to answer the Pipeline Parameters. Not sure there is a good way to set internal task parameters. If you want some connectivity there, I imagine you write a Pipeline parameter and then your Task Parameter references the Pipeline Parameter for execution.

kind: PipelineRun
...
spec:
  params:
    - name: 'foo'
      description: 'Must be given before I can start'
  # this is probably nested under pipelineSpec or something
  tasks:
    - name: 'my-first-task'
      params:
        - name: 'bar'
          value: '$(params.foo)' # at runtime, this will be the value supplied to the param `foo`
... 

Something like that... this was typed without testing, so refer to the Tekton docs to make sure you get the right syntax when you're testing it out.

I may have misunderstood your question from Slack, so apologizes for that. Let me know if this clears things up.

yuanchi2807 commented 1 year ago

@andrewballantyne Using SDK v1, can I expect each func_to_container_op or load_component_from_file from kfp.components to have its own configuration section on the 'Create run' screen? Will try a couple of toy cases to confirm. Thanks.

andrewballantyne commented 1 year ago

@yuanchi2807 I can't speak much about the SDK in play here. @HumairAK @harshad16 might know more on that front.

I'm purely speaking from a Tekton point of view (the run aspect of DSP). It is my understanding that there is only 1 layer of input parameters, and that is of the Pipeline Run (defined from the Pipeline)

roytman commented 1 year ago

@yuanchi2807 , as a temporary workaround, we can use the fact that ODH GUI presents RUN input parameters according to ABC, so we might define a name convention for input parameters.

For example, ALL default Global parameters can have a prefix: global

image

All parameters for step A and the English language can have a prefix A_eng, the same step for Spanish will have a prefix A_sp, or something similar.

image

In the code, we can guarantee that a step-specific parameter has precedence. In order to reduce mistake probabilities, we can add some checks of input parameter names.

It is not an ideal solution, but this approach allows us to define global parameters while also allowing for overwriting them on a per-step (nested step) basis. Furthermore, the ODH GUI will consolidate the step-related parameters.

andrewballantyne commented 1 year ago

How one wires up the Pipeline itself is entirely up to the creator of the Pipeline. But I don't think the Dashboard UI should help this out in anyway. We show top-level Pipeline params, and let the configuration manage itself imo.

Am I to understand that this is an issue we can close? This sounds data-driven and not something that the UI will manage outside of what it is currently doing today.

yuanchi2807 commented 1 year ago

@andrewballantyne What @roytman proposed workaround works. This issue is certainly not as high priority as flowing logs back to DSP dashboard. Can reopen later.

andrewballantyne commented 1 year ago

We spoke more about the use-case here and it has come to my attention we are not talking about a small number of parameters for their use-case. Although yes, this seems to be the design intent by Tekton parameters, it seems it's a bit clunky in our UI. They have several dozens of parameters they are trying to inject into the two tasks they have, and in order to manage that they are prefixing their values and associating them in the Pipeline as expected (and noted in our previous conversations on this ticket).

This looks very wonky in our UI having to scroll through 40 some odd parameters dealing with all the various entires of partial relationship. I wonder if we can find a better way, maybe a grouping mechanism or something to help with large input-based parameters; some metadata we can adhere to help make collapsible sections.

@yannnz @kywalker-rh We might need to meet about this and discuss an approach to solving this from a UX point of view.

@HumairAK @gmfrasca we should consider investigating kubeflow pipelines for any existing functionality that would be expected that we follow. Your help here would be appreciated.

cc @jedemo FYI

yuanchi2807 commented 1 year ago

Another workaround proposed by @blublinsky is to capture the dozen or so parameters as JSON. So at pipeline input page, several chunks of JSON are inserted by user. Not sure if they are consistent with UI design.

andrewballantyne commented 1 year ago

Another workaround proposed by @blublinsky is to capture the dozen or so parameters as JSON. So at pipeline input page, several chunks of JSON are inserted by user. Not sure if they are consistent with UI design.

This has a limitation I suspect -- we use inputs and not text areas -- so a large JSON blob will effectively be unmanageable in our UI as well.

dgutride commented 9 months ago

Migrated to: https://issues.redhat.com/browse/RHOAIENG-569