determined-ai / determined

Determined is an open-source machine learning platform that simplifies distributed training, hyperparameter tuning, experiment tracking, and resource management. Works with PyTorch and TensorFlow.
https://determined.ai
Apache License 2.0
3k stars 350 forks source link

fix: comparison view parallel coordinates chart shouldn't break when selecting rows [ET-261] #9584

Closed EmilyBonar closed 3 months ago

EmilyBonar commented 3 months ago

Ticket

ET-261

Description

Test Plan

Go to the Glide experiment list, select an experiment, open comparison view, and switch to the Hyperparameters tab to view the Parallel Coordinates plot. From there, select and unselect various experiments with different hyperparameters and metrics. At no point should the selected metric change unless you unselect the only experiment that has it. At no point should all of the lines on the chart disappear.

Checklist

netlify[bot] commented 3 months ago

Deploy Preview for determined-ui ready!

Name Link
Latest commit e9df0aaa71fca039eac796f0ef0cc1104b442c2e
Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/66857d04e81d6e00089dc4f1
Deploy Preview https://deploy-preview-9584--determined-ui.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

johnkim-det commented 3 months ago

testing on netlify, I think I'm still seeing some cases where selecting additional experiments or runs removes previously charted ones. are there cases where we would still expect something like this to happen?

https://github.com/determined-ai/determined/assets/97752292/f3b8a647-0dfc-43e3-8010-403ac4501f88

https://github.com/determined-ai/determined/assets/97752292/59a4f070-07a1-420a-84c4-a9e7e6a6e836

EmilyBonar commented 3 months ago

testing on netlify, I think I'm still seeing some cases where selecting additional experiments or runs removes previously charted ones. are there cases where we would still expect something like this to happen?

No, that shouldn't ever happen. I think I fixed the issue, give it a try and let me know.

EmilyBonar commented 3 months ago

@johnkim-det Ok try it again. I'll remove the console.logs once we can confirm it works. And yes, that message is expected in the last screenshot.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 97.42120% with 9 lines in your changes missing coverage. Please review.

Project coverage is 47.14%. Comparing base (3095703) to head (e9df0aa).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #9584 +/- ## ========================================== - Coverage 51.93% 47.14% -4.80% ========================================== Files 1255 932 -323 Lines 152817 123933 -28884 Branches 3120 3123 +3 ========================================== - Hits 79372 58429 -20943 + Misses 73290 65349 -7941 Partials 155 155 ``` | [Flag](https://app.codecov.io/gh/determined-ai/determined/pull/9584/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Δ | | |---|---|---| | [harness](https://app.codecov.io/gh/determined-ai/determined/pull/9584/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `?` | | | [web](https://app.codecov.io/gh/determined-ai/determined/pull/9584/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `49.44% <97.42%> (+0.19%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/determined-ai/determined/pull/9584?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Δ | | |---|---|---| | [...rc/components/CompareHyperparameters.test.mock.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FCompareHyperparameters.test.mock.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29tcGFyZUh5cGVycGFyYW1ldGVycy50ZXN0Lm1vY2sudHN4) | `100.00% <100.00%> (ø)` | | | [...ui/react/src/components/CompareHyperparameters.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FCompareHyperparameters.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29tcGFyZUh5cGVycGFyYW1ldGVycy50c3g=) | `92.41% <100.00%> (+0.20%)` | :arrow_up: | | [...eact/src/components/CompareParallelCoordinates.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FCompareParallelCoordinates.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29tcGFyZVBhcmFsbGVsQ29vcmRpbmF0ZXMudHN4) | `92.83% <100.00%> (+4.22%)` | :arrow_up: | | [.../react/src/components/ComparisonView.test.mock.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FComparisonView.test.mock.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29tcGFyaXNvblZpZXcudGVzdC5tb2NrLnRzeA==) | `99.32% <100.00%> (+0.01%)` | :arrow_up: | | [webui/react/src/components/ComparisonView.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FComparisonView.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29tcGFyaXNvblZpZXcudHN4) | `97.79% <100.00%> (+0.01%)` | :arrow_up: | | [webui/react/src/types.ts](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Ftypes.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL3R5cGVzLnRz) | `99.68% <100.00%> (ø)` | | | [webui/react/src/utils/data.ts](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Futils%2Fdata.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL3V0aWxzL2RhdGEudHM=) | `92.94% <100.00%> (ø)` | | | [webui/react/src/utils/tests/generateTestData.ts](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Futils%2Ftests%2FgenerateTestData.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL3V0aWxzL3Rlc3RzL2dlbmVyYXRlVGVzdERhdGEudHM=) | `92.00% <100.00%> (+0.88%)` | :arrow_up: | | [...bui/react/src/pages/F\_ExpList/F\_ExperimentList.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fpages%2FF_ExpList%2FF_ExperimentList.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL3BhZ2VzL0ZfRXhwTGlzdC9GX0V4cGVyaW1lbnRMaXN0LnRzeA==) | `0.00% <0.00%> (ø)` | | | [webui/react/src/pages/FlatRuns/FlatRuns.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fpages%2FFlatRuns%2FFlatRuns.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL3BhZ2VzL0ZsYXRSdW5zL0ZsYXRSdW5zLnRzeA==) | `10.57% <0.00%> (-0.01%)` | :arrow_down: | | ... and [1 more](https://app.codecov.io/gh/determined-ai/determined/pull/9584?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | | ... and [326 files with indirect coverage changes](https://app.codecov.io/gh/determined-ai/determined/pull/9584/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai)