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

Pipeline metrics are always sorted alphabetically #5215

Closed
ypitrey opened this issue Feb 12, 2021 · 6 comments · Fixed by #5701
Closed

Pipeline metrics are always sorted alphabetically #5215

ypitrey opened this issue Feb 12, 2021 · 6 comments · Fixed by #5701
Labels
good first issue help wanted The community is welcome to contribute. kind/feature

Comments

@ypitrey
Copy link

ypitrey commented Feb 12, 2021

/kind bug

I have a pipeline that exposes pipeline metrics following this example. It's working great, but it seems that the metrics always appear sorted alphabetically. I would like to make use of the fact that the first two metrics are displayed next to each run in the run list view, but the two metrics I'm interested in are not the first two in alphabetical order.

Note: I need to expose more than these two metrics, as I am using these metrics to aggregate results across multiple runs.

Am I doing something wrong? Is there a way to not sort the metrics alphabetically?

For context, here is the code I'm using to expose the metrics:

def write_metrics_data(kpis: dict):
    metrics = {
        'metrics': [
            {
                'name': name,
                'numberValue': value,
                'format': "PERCENTAGE" if 'percent' in name else "RAW",
            }
            for name, value in kpis.items()
        ]
    }
    myjson.write(metrics, Path('/mlpipeline-metrics.json'))

and this is the header row as displayed in the Run Output tab:

activeTrips | dropsPerHour | idleHours | kilometresPerDrop | kilometresPerHour | percentReloads | ...

which seems to be in alphabetical order. However, if I do:

print(kpis.keys())

I get this:

['dropsPerHour', 'unscheduled', 'activeTrips', 'totalHours', ...]

which isn't in alphabetical order. And the two metrics I'm interested in are the first two ones in this list.

Environment:

  • Kubeflow version: 1.0.4
@davidspek
Copy link
Contributor

/area pipelines
@Bobgy maybe you can move this over to the pipelines repo

@ypitrey
Copy link
Author

ypitrey commented Feb 22, 2021

Any updates on this issue? I'll have to name my metrics 1. dropsPerHour and 2. unscheduled for them to appear at the top of the list for now, but I fear the gods of software design are going to be angry with me. 😄

Thanks guys

@Bobgy Bobgy transferred this issue from kubeflow/kubeflow Mar 1, 2021
@Bobgy
Copy link
Contributor

Bobgy commented Mar 1, 2021

We are open for contributions, here's frontend development guide: https://github.com/kubeflow/pipelines/tree/master/frontend.

And I believe related code to this issue is in

{...CompareUtils.singleRunToMetricsCompareProps(runMetadata, workflow)}
.

@Bobgy Bobgy added good first issue help wanted The community is welcome to contribute. kind/feature and removed kind/bug labels Mar 1, 2021
@annajung
Copy link
Member

Hi @Bobgy @ypitrey, a new contributor here looking for a good first issue to work on.
I like to work on this issue if this is still relevant & have a few questions to clarify the ask.

Is the ask to simply remove the sorting of metrics or to create a functionality that a user can choose the metrics that show up on the UI?

Note: I need to expose more than these two metrics, as I am using these metrics to aggregate results across multiple runs.

In addition, I think you're looking for a way to show more than two metrics? Should this be part of this issue?

@Bobgy
Copy link
Contributor

Bobgy commented May 19, 2021

Thank you @annajung !
This is still relevant, it seems removing sorting would be enough.

@annajung
Copy link
Member

Thanks for the clarification @Bobgy
Created a PR #5701, any feedback would be appreciated! Thanks

google-oss-robot pushed a commit that referenced this issue May 27, 2021
#5215 (#5701)

* Remove alphabetical sorting of the metrics column

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Fix code format

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Refactor test to use data-testid

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Remove snapshot in favor of text comparison

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
Bobgy pushed a commit that referenced this issue Jun 21, 2021
#5215 (#5701)

* Remove alphabetical sorting of the metrics column

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Fix code format

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Refactor test to use data-testid

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Remove snapshot in favor of text comparison

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
Bobgy pushed a commit that referenced this issue Jun 21, 2021
#5215 (#5701)

* Remove alphabetical sorting of the metrics column

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Fix code format

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Refactor test to use data-testid

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Remove snapshot in favor of text comparison

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue help wanted The community is welcome to contribute. kind/feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants