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

Fix or remove metrics timeline in metadata panel #1000

Closed
antonymilne opened this issue Aug 3, 2022 · 9 comments
Closed

Fix or remove metrics timeline in metadata panel #1000

antonymilne opened this issue Aug 3, 2022 · 9 comments
Assignees
Labels
Design: Visual Design Javascript Pull requests that update Javascript code Python Pull requests that update Python code

Comments

@antonymilne
Copy link
Contributor

antonymilne commented Aug 3, 2022

This plot in the metadata panel is what I'm talking about:
image

The function that generates this, models.flowchart.DataNodeMetadata.load_versioned_tracking_data, has a few problems:

  1. Most important: it doesn't work when metrics are stored anywhere outside the local file system (e.g. s3). I haven't tested but I believe that as well as not showing the plot, this will also stop a user from seeing other bits of information in the metadata panel like the latest run metric
  2. Small bug: if you have a timestamped folder which is empty then you'll get FileNotFoundError: [Errno 2] No such file or directory: '/Users/antony_milne/kedro_stuff/kedro-viz/demo-project/data/09_tracking/linear_score.json/2022-08-03T22.06.41.403Z/linear_score.json' and again the metadata panel won't show stuff. This error should have been swallowed and shouldn't be allowed to break other things
  3. It needs refactoring (e.g. to use the new loading functions in data_loader.py)

We should address the above problems with the function or just remove this plot altogether. Why might we prefer to remove it?

  • at some point we will have a more holistic way of comparing metrics (Exploring Metrics on Experiment Tracking - User Testing Synthesis  kedro#1627) that lives outside the metadata side panel and probably renders this plot redundant. The current metadata panel seems to be generally regarded as not easily discoverable
  • the plot itself is quite a partial solution in its current form, e.g. doesn't cope well with metrics of differing scales, doesn't have uniform time spacing, which makes its use limited in practice
  • the code related to this currently the only place in kedro-viz where we use pandas and plotly. If we don't have load_versioned_tracking_data then we don't have create_metrics_plot. Hence we could then remove pandas and plotly as kedro-viz dependencies altogether and there'd be no need for the rewrite suggested in Remove pandas and plotly dependency #999

I don't think it's urgent to remove it, but if we do plan on doing so at some point in future there's not much point putting time into fixing the bugs and refactoring it, since they're not showstoppers at the moment. So what we do here basically depends on where kedro-org/kedro#1627 ends up and whether there will be a need for this plot at all in the future.

@antonymilne antonymilne changed the title Fix (or remove) metrics timeline in metadata panel Fix or remove metrics timeline in metadata panel Aug 3, 2022
@antonymilne
Copy link
Contributor Author

Also... we've reached the 1000 issue milestone! Do I get a prize?

@rashidakanchwala
Copy link
Contributor

Should we remove this now that we have everything in experiement tracking?

@antonymilne
Copy link
Contributor Author

In my opinion yes, and then we can also do this at the same time: #999

@tynandebold
Copy link
Member

I'm not sure we should yet. We spoke of a user flow wherein someone would see a metrics plot in the metadata panel which would then lead them onto and into the experiment tracking page, where they could further explore their data. In this flow, the metadata plot would be the conduit between the two (requiring some modification to become a link, perhaps).

@AntonyMilneQB what do you think of that? Would it be beneficial and add value?

@antonymilne
Copy link
Contributor Author

This makes sense as a feature, but we would need to (a) fix the plot as per the above points and probably also (b) make the graph consistent with how it displays in experiment tracking. In my opinion providing a link to experiment tracking page would be essential and the same logic would be used for any other tracked datasets like plots. Compared to showing plot previews in the metadata side panel, I think the metrics timeline is much less valuable. So maybe we should start by trying to link plots in the metadata side panel to the experiments tracking page?

Overall does the added value of the feature outweigh the work involved? Maybe, maybe not. Off the top of my head I think it would be at least 5 points of work on the backend anyway.

@rashidakanchwala
Copy link
Contributor

I was also thinking that this metrics timeline looks too basic now compared to the experiment tracking one.

@rashidakanchwala
Copy link
Contributor

rashidakanchwala commented Jan 30, 2023

So based on the discussions we had in the grooming session, the conclusion is that we need to fix the backend for this and also improved the design.

For the design I was thinking since we already have the d3 metrics timeline component ready -- we could just use that? The timeline will adjust itself based on the width and we show only last 10 runs so it don't think it will appear squashed. And maybe the 'Expand' button will take us to the main plot in experiment tracking.

@rashidakanchwala rashidakanchwala added the Javascript Pull requests that update Javascript code label Jan 30, 2023
@tynandebold
Copy link
Member

So maybe we should start by trying to link plots in the metadata side panel to the experiments tracking page?

I think we should start with this, as you said, and probably remove the metrics plots in the metadata panel. We'll try and get this in next sprint.

@tynandebold tynandebold moved this from Backlog to Todo in Kedro-Viz Feb 20, 2023
@rashidakanchwala rashidakanchwala moved this from Todo to In Progress in Kedro-Viz Feb 22, 2023
@rashidakanchwala rashidakanchwala self-assigned this Feb 22, 2023
@rashidakanchwala rashidakanchwala moved this from In Progress to In Review in Kedro-Viz Feb 28, 2023
@rashidakanchwala rashidakanchwala moved this from In Review to Done in Kedro-Viz Mar 3, 2023
@tynandebold
Copy link
Member

This is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design: Visual Design Javascript Pull requests that update Javascript code Python Pull requests that update Python code
Projects
Status: Done
Development

No branches or pull requests

3 participants