Add support for system metrics #33
Conversation
| 'system/disk_available_megabytes': 'Disk Available (MB)', | ||
| 'system/disk_usage_megabytes': 'Disk (MB)', | ||
| 'system/disk_usage_percentage': 'Disk (%)', | ||
| 'system/gpu_0_memory_usage_megabytes': 'GPU 0 Memory (MB)', |
There was a problem hiding this comment.
Couldn't this be built more cleanly with an iterator?
There was a problem hiding this comment.
I am not an expert in typescript, so I don't know 😞
I wanted to make sure we can ship this!
| @@ -146,7 +146,7 @@ function processRunBatchData( | |||
| }), | |||
| sortKey: `${run.name}${contextName}`, | |||
| }; | |||
| if (run.name.startsWith('__system__')) { | |||
| if (run.name.startsWith('system/')) { | |||
There was a problem hiding this comment.
This is the third time I'm seeing this magic string in this PR - could we extract a constant somehow?
There was a problem hiding this comment.
This is based on the existing UI code from Aim at this stage. Extracting a constant would make total sense, but I wanted to keep this as close as possible to the current upstream logic.
| 'system/disk_available_megabytes': string; | ||
| 'system/disk_usage_megabytes': string; | ||
| 'system/disk_usage_percentage': string; | ||
| 'system/gpu_0_memory_usage_megabytes': string; |
There was a problem hiding this comment.
Same comment re: tidying this w/an iterator...
There was a problem hiding this comment.
same reply as above: if there is a way to do this cleanly in typescript, I'll take it!
This PR enables support for system metrics by reverting some of the code removals we did in #31 (based on #3), adapting them for MLFlow keys (starting with
system/instead of__system__and covering different aspects), and fixing the key ordering (which was broken in the UI and relied on the server originally).Fixes G-Research/fasttrackml#536.